Skip to content

Fixes #27062: Fix silent exception swallowing in FeedRepository and resource leaks in SchemaFieldExtractor#27063

Open
RajdeepKushwaha5 wants to merge 1 commit intoopen-metadata:mainfrom
RajdeepKushwaha5:fix/feed-cleanup-logging-and-schema-resource-leak
Open

Fixes #27062: Fix silent exception swallowing in FeedRepository and resource leaks in SchemaFieldExtractor#27063
RajdeepKushwaha5 wants to merge 1 commit intoopen-metadata:mainfrom
RajdeepKushwaha5:fix/feed-cleanup-logging-and-schema-resource-leak

Conversation

@RajdeepKushwaha5
Copy link
Copy Markdown
Contributor

@RajdeepKushwaha5 RajdeepKushwaha5 commented Apr 6, 2026

Describe your changes:

Fixes #27062

Two related code quality fixes in openmetadata-service that improve debuggability and resource management:

Fix 1: FeedRepository.deleteByAbout() — silent exception swallowing

The deleteByAbout() method had a completely empty catch block that silently swallowed all exceptions during thread deletion:

// Before
catch (Exception ex) {
    // Continue deletion
}

// After
catch (Exception ex) {
    LOG.warn("Failed to delete thread {} during entity {} cleanup", threadId, entityId, ex);
}

The class already uses @Slf4j with LOG throughout (6 existing usages), making this empty catch block a clear oversight. When thread deletion fails during entity cleanup, operators now get actionable diagnostic information instead of silent data orphaning.

Fix 2: SchemaFieldExtractor.loadMainSchema() & loadSchema() — unclosed InputStreams

Both methods opened InputStream via getClassLoader().getResourceAsStream() but never closed them, leaking file descriptors on each schema load. Fixed by wrapping with try-with-resources:

// Before
InputStream schemaInputStream = getClass().getClassLoader().getResourceAsStream(schemaPath);
// ... used but never closed

// After
try (InputStream schemaInputStream = getClass().getClassLoader().getResourceAsStream(schemaPath)) {
    // ... automatically closed
}

Notably, getAllEntityTypes() in the same file already uses try-with-resources correctly — this fix makes the file internally consistent.

Testing: Both fixes are behavioral no-ops (logging-only change + resource lifecycle change) with no functional changes to control flow. Validated with mvn spotless:check.

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.
  • I have added a test that covers the exact scenario we are fixing. For complex issues, comment the issue number in the test for future reference.

Copilot AI review requested due to automatic review settings April 6, 2026 00:19
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses two backend code-quality issues in openmetadata-service to improve operational debuggability and prevent resource leaks during JSON schema loading and entity cleanup.

Changes:

  • Wraps schema resource InputStream usage in SchemaFieldExtractor with try-with-resources to ensure streams are closed.
  • Stops silently swallowing exceptions in FeedRepository.deleteByAbout() by emitting a WARN log when thread deletion fails.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
openmetadata-service/src/main/java/org/openmetadata/service/util/SchemaFieldExtractor.java Ensures schema-loading InputStreams are closed via try-with-resources; refactors exception handling around schema load.
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/FeedRepository.java Logs failures when deleting threads during entity cleanup instead of swallowing exceptions silently.
Comments suppressed due to low confidence (2)

openmetadata-service/src/main/java/org/openmetadata/service/util/SchemaFieldExtractor.java:192

  • In this catch-all branch, the log message drops the stack trace (only logs e.getMessage()), and the new SchemaProcessingException is created without the original Throwable as its cause. This makes schema load failures much harder to debug. Consider logging the exception itself and using the SchemaProcessingException(String message, Throwable cause, ErrorType) constructor so the root cause is preserved.
    } catch (Exception e) {
      LOG.error("Error loading schema '{}': {}", schemaPath, e.getMessage());
      throw new SchemaProcessingException(
          "Error loading schema '" + schemaPath + "': " + e.getMessage(),
          SchemaProcessingException.ErrorType.OTHER);

openmetadata-service/src/main/java/org/openmetadata/service/util/SchemaFieldExtractor.java:444

  • Same as above: this logs only e.getMessage() and wraps the error without setting the original exception as the cause. To retain actionable diagnostics, log the exception (stack trace) and pass e into the SchemaProcessingException constructor that accepts a cause.
    } catch (Exception e) {
      LOG.error("Error loading schema '{}': {}", schemaPath, e.getMessage());
      throw new SchemaProcessingException(
          "Error loading schema '" + schemaPath + "': " + e.getMessage(),
          SchemaProcessingException.ErrorType.OTHER);

…in SchemaFieldExtractor

- FeedRepository.deleteByAbout(): Replace empty catch block with LOG.warn
  to log failed thread deletions during entity cleanup with thread ID,
  entity ID, and exception details

- SchemaFieldExtractor.loadMainSchema(): Wrap InputStream in
  try-with-resources to prevent file descriptor leak

- SchemaFieldExtractor.loadSchema(): Same try-with-resources fix,
  consistent with getAllEntityTypes() which already does this correctly
@RajdeepKushwaha5 RajdeepKushwaha5 force-pushed the fix/feed-cleanup-logging-and-schema-resource-leak branch from d429cdd to 64624a0 Compare April 6, 2026 00:24
@RajdeepKushwaha5 RajdeepKushwaha5 requested a review from Copilot April 6, 2026 00:24
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@gitar-bot
Copy link
Copy Markdown

gitar-bot bot commented Apr 6, 2026

Code Review ✅ Approved

Fixes silent exception swallowing in FeedRepository and resource leaks in SchemaFieldExtractor to improve error visibility and prevent resource exhaustion. No issues found.

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Silent Exception Swallowing & Resource Leaks in Backend Services

2 participants