Skip to content

Log Gracefully and ignore entity id null error#27459

Open
mohityadav766 wants to merge 4 commits intomainfrom
search-error-log
Open

Log Gracefully and ignore entity id null error#27459
mohityadav766 wants to merge 4 commits intomainfrom
search-error-log

Conversation

@mohityadav766
Copy link
Copy Markdown
Member

Describe your changes:

Fixes

I worked on ... because ...

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.

@mohityadav766 mohityadav766 self-assigned this Apr 17, 2026
Copilot AI review requested due to automatic review settings April 17, 2026 08:33
@github-actions github-actions bot added backend safe to test Add this label to run secure Github workflows on PRs labels Apr 17, 2026
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 aims to reduce noisy/avoidable errors by (1) logging WebSocket disconnect-related exceptions more gracefully and (2) avoiding failures when recording search-index reader errors that don’t have a usable entity id.

Changes:

  • Update Jetty 12 WebSocket error handling to downgrade ClosedChannelException logging and add safer logging around error handling.
  • Update distributed search indexing PartitionWorker to derive entityId more intentionally (prefer EntityInterface.getId()) and skip recording reader failures when entityId is null.

Reviewed changes

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

File Description
openmetadata-service/src/main/java/org/openmetadata/service/socket/Jetty12WebSocketHandler.java Adjusts WebSocket error logging to treat closed-channel errors as non-actionable noise and wraps error handling defensively.
openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/distributed/PartitionWorker.java Changes how reader failures derive entityId and adds a skip path when an id can’t be determined.

Copilot AI review requested due to automatic review settings April 17, 2026 09:41
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 4 out of 4 changed files in this pull request and generated no new comments.

@gitar-bot
Copy link
Copy Markdown

gitar-bot bot commented Apr 17, 2026

Code Review ✅ Approved 1 resolved / 1 findings

Graceful logging implemented for null entity IDs, resolving the panic caused by emitting messages on closed channels. No further issues found.

✅ 1 resolved
Bug: emit() called on closed channel with null message

📄 openmetadata-service/src/main/java/org/openmetadata/service/socket/Jetty12WebSocketHandler.java:131-141
After detecting a ClosedChannelException (line 132-133), the code still falls through to emit("error", "websocket error", error.getMessage()) at line 141. For ClosedChannelException, getMessage() typically returns null, and emitting on a connection whose channel is already closed is unnecessary and may itself throw an exception (caught by the outer try-catch, but avoidable). The emit call should be inside the else block or guarded by a condition.

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqubecloud
Copy link
Copy Markdown

@github-actions
Copy link
Copy Markdown
Contributor

🔴 Playwright Results — 1 failure(s), 26 flaky

✅ 3658 passed · ❌ 1 failed · 🟡 26 flaky · ⏭️ 89 skipped

Shard Passed Failed Flaky Skipped
🟡 Shard 1 477 0 3 4
🟡 Shard 2 647 0 4 7
🟡 Shard 3 656 0 4 1
🟡 Shard 4 629 0 5 27
🔴 Shard 5 610 1 0 42
🟡 Shard 6 639 0 10 8

Genuine Failures (failed on all attempts)

Pages/Glossary.spec.ts › Add and Remove Assets (shard 5)
�[31mTest timeout of 180000ms exceeded.�[39m
🟡 26 flaky test(s) (passed on retry)
  • Features/DataAssetRulesDisabled.spec.ts › Verify the ApiEndpoint entity item action after rules disabled (shard 1, 1 retry)
  • Features/CustomizeDetailPage.spec.ts › Data Product - customization should work (shard 1, 1 retry)
  • Pages/UserCreationWithPersona.spec.ts › Create user with persona and verify on profile (shard 1, 1 retry)
  • Features/BulkEditEntity.spec.ts › Glossary (shard 2, 1 retry)
  • Features/ChangeSummaryBadge.spec.ts › Automated badge should appear on entity description with Automated source (shard 2, 1 retry)
  • Features/DataQuality/TestCaseImportExportE2eFlow.spec.ts › Admin: Complete export-import-validate flow (shard 2, 1 retry)
  • Features/DataQuality/TestCaseResultPermissions.spec.ts › User with only VIEW cannot PATCH results (shard 2, 1 retry)
  • Features/RestoreEntityInheritedFields.spec.ts › Validate restore with Inherited domain and data products assigned (shard 3, 1 retry)
  • Features/RestoreEntityInheritedFields.spec.ts › Validate restore with Inherited domain and data products assigned (shard 3, 1 retry)
  • Features/SettingsNavigationPage.spec.ts › should show navigation blocker when leaving with unsaved changes (shard 3, 1 retry)
  • Features/UserProfileOnlineStatus.spec.ts › Should show online status badge on user profile for active users (shard 3, 1 retry)
  • Pages/Customproperties-part2.spec.ts › entityReferenceList shows item count, scrollable list, no expand toggle (shard 4, 1 retry)
  • Pages/DataContracts.spec.ts › Create Data Contract and validate for DashboardDataModel (shard 4, 1 retry)
  • Pages/DataProductAndSubdomains.spec.ts › Add tags to data product via UI (shard 4, 1 retry)
  • Pages/DomainUIInteractions.spec.ts › Delete domain with subdomains shows warning (shard 4, 1 retry)
  • Pages/Entity.spec.ts › Glossary Term Add, Update and Remove (shard 4, 1 retry)
  • Pages/Glossary.spec.ts › Create term with related terms, tags and owners during creation (shard 6, 1 retry)
  • Pages/HyperlinkCustomProperty.spec.ts › should show No Data placeholder when hyperlink has no value (shard 6, 1 retry)
  • Pages/Lineage/DataAssetLineage.spec.ts › verify create lineage for entity - Container (shard 6, 1 retry)
  • Pages/Lineage/DataAssetLineage.spec.ts › verify create lineage for entity - Data Model (shard 6, 1 retry)
  • Pages/Lineage/LineageFilters.spec.ts › Verify lineage schema filter selection (shard 6, 1 retry)
  • Pages/Lineage/LineageRightPanel.spec.ts › Verify custom properties tab IS visible for supported type: searchIndex (shard 6, 1 retry)
  • Pages/Login.spec.ts › Refresh should work (shard 6, 2 retries)
  • Pages/ODCSImportExport.spec.ts › Multi-object ODCS contract - object selector shows all schema objects (shard 6, 1 retry)
  • Pages/UserDetails.spec.ts › Admin user can edit teams from the user profile (shard 6, 1 retry)
  • Pages/Users.spec.ts › Permissions for table details page for Data Consumer (shard 6, 1 retry)

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

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

Labels

backend safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants