Skip to content

feat: Supports callbacks when reading a message fails#331

Merged
tigrannajaryan merged 1 commit into
open-telemetry:mainfrom
tttoad:feat-error-cb
Feb 21, 2025
Merged

feat: Supports callbacks when reading a message fails#331
tigrannajaryan merged 1 commit into
open-telemetry:mainfrom
tttoad:feat-error-cb

Conversation

@tttoad

@tttoad tttoad commented Dec 27, 2024 •
edited by tigrannajaryan
Loading

Copy link
Copy Markdown
Contributor

Resolves #330

This adds a new Server callback OnReadMessageError that is called when a read or deserialization error occurs.

@tttoad
tttoad requested a review from a team as a code owner December 27, 2024 07:32
@linux-foundation-easycla

linux-foundation-easycla Bot commented Dec 27, 2024 •
edited
Loading

Copy link
Copy Markdown

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: tttoad / name: ttoad (5c2f281)

@tigrannajaryan

tigrannajaryan commented Jan 7, 2025

Copy link
Copy Markdown
Member

Where is the new callback used?

@tttoad

tttoad commented Jan 8, 2025

Copy link
Copy Markdown
Contributor Author

Where is the new callback used?

Sorry, missed submitting a file change.
PTAL.

@tigrannajaryan

tigrannajaryan commented Feb 7, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

This appears stale - no response on comments. (Nevermind, I failed to hit Publish button with my question).

This conflicts with other changes to callbacks. Please recreate if still relevant.

@tigrannajaryan tigrannajaryan reopened this Feb 7, 2025
tigrannajaryan
tigrannajaryan reviewed Feb 7, 2025
View reviewed changes
Comment thread server/serverimpl.go Outdated
tigrannajaryan
tigrannajaryan reviewed Feb 8, 2025
View reviewed changes
Comment thread server/types/callbacks.go Outdated
Comment thread server/serverimpl_test.go Outdated
Comment thread server/serverimpl_test.go Outdated
Comment thread server/serverimpl.go
@codecov

codecov Bot commented Feb 8, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 70.37037% with 8 lines in your changes missing coverage. Please review.

Project coverage is 78.46%. Comparing base (2ecac8d) to head (5c2f281).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
server/serverimpl.go 65.21% 6 Missing and 2 partials ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #331      +/-   ##
==========================================
+ Coverage   78.29%   78.46%   +0.17%     
==========================================
  Files          25       25              
  Lines        2386     2401      +15     
==========================================
+ Hits         1868     1884      +16     
  Misses        410      410              
+ Partials      108      107       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tigrannajaryan

tigrannajaryan commented Feb 19, 2025

Copy link
Copy Markdown
Member

@tttoad please fix the build.

@tttoad
tttoad force-pushed the feat-error-cb branch 2 times, most recently from bd7e659 to 25fc858 Compare February 20, 2025 02:36
@tttoad

tttoad commented Feb 20, 2025

Copy link
Copy Markdown
Contributor Author

@tigrannajaryan PTAL.

tigrannajaryan
tigrannajaryan reviewed Feb 20, 2025
View reviewed changes
Comment thread server/serverimpl_test.go Outdated
tigrannajaryan
tigrannajaryan approved these changes Feb 20, 2025
View reviewed changes

@tigrannajaryan tigrannajaryan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Just one minor suggestion to improve the comment and we can merge.

@tigrannajaryan
tigrannajaryan merged commit e718373 into open-telemetry:main Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tigrannajaryan tigrannajaryan tigrannajaryan approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Feature] Callback on message read error

2 participants

@tttoad @tigrannajaryan