Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't record GeneratorExit errors in stream RPC spans #129

Merged

Conversation

cbrewster
Copy link
Member

Why

Stream RPCs which are not read to completion would show up with a GeneratorExit error in their associated span. There are valid cases where a caller may not need to exhaustively read the stream, so these should not be considered errors.

What changed

  • Stop using the span as a context manager, we'll handle recording errors ourselves
  • Catch RiverExceptions, Exceptions, and CancelledErrors
  • Switch from AsyncIterator to AsyncGenerator for stream return types
    • This allows us to call close on the generator
    • We may also want to do this in the codegen, this will be especially useful for cancellation support (just close the generator)

Test plan

  • Added a test which ensured a closed generator creates a span with OK status

@cbrewster
Copy link
Member Author

cbrewster commented Dec 3, 2024

Current dependencies on/for this PR:

This comment was autogenerated by Freephite.

@cbrewster cbrewster requested a review from a team as a code owner December 3, 2024 21:53
@cbrewster cbrewster requested review from blast-hardcheese and removed request for a team December 3, 2024 21:53
@cbrewster cbrewster added bug Something isn't working patch Bump patch version labels Dec 3, 2024
@cbrewster cbrewster force-pushed the 12-03-Don_t_record_GeneratorExit_errors_in_stream_RPC_spans branch from 855d03f to 8533285 Compare December 3, 2024 22:18
@cbrewster cbrewster force-pushed the 12-03-Don_t_record_GeneratorExit_errors_in_stream_RPC_spans branch from 8533285 to 109c052 Compare December 3, 2024 22:19
@cbrewster cbrewster merged commit 92c3da0 into main Dec 5, 2024
3 checks passed
@cbrewster cbrewster deleted the 12-03-Don_t_record_GeneratorExit_errors_in_stream_RPC_spans branch December 5, 2024 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working patch Bump patch version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants