Defer trailer transformer callbacks on null msg-body to subscription. #1577
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Motivation: When we have a null msg-body and call transform, then the callbacks of the trailer transformer are invoked on the calling thread rather than the subsciption. Modifications: Used defer to defer the invocation. Result: Better error handling in case of erroneous trailer transformer (ie. trailers validations) and deferred execution of the callbacks.
There was a problem hiding this comment.
I looked at the CI failures: https://github.com/apple/servicetalk/pull/1577/checks?check_run_id=2666777501
Never saw ConnectionCloseHeaderHandlingTest$NonPipelinedRequestsTest being flaky before this PR. Let's hold it for now?
|
Build failure due #1579 |
|
@idelpivnitskiy as we discussed offline, we have seen this error again outside the context of this change. |
bondolo
pushed a commit
to bondolo/servicetalk
that referenced
this pull request
Jun 10, 2021
…pple#1577) Motivation: When we have a null msg-body and call `transform`, then the callbacks of the trailer transformer are invoked on the calling thread rather than the subscriber thread. Modifications: - Used `Publisher#defer` to defer the invocation of `TrailersTransformer#payloadComplete`; Result: Better error handling in case of erroneous trailer transformer (ie. trailers validations) and deferred execution of the callbacks until someone subscribers to payload body.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation:
When we have a null msg-body and call
transform, then the callbacks of the trailertransformer are invoked on the calling thread rather than the subscriber thread.
Modifications:
Publisher#deferto defer the invocation ofTrailersTransformer#payloadComplete;Result:
Better error handling in case of erroneous trailer transformer (ie. trailers validations)
and deferred execution of the callbacks until someone subscribes to the payload body.
Fix: #1576