Skip to content

Call throw and return on response iterables in the node adaptor#775

Merged
srikrsna-buf merged 4 commits intomainfrom
sk/throw-return-iterables
Aug 24, 2023
Merged

Call throw and return on response iterables in the node adaptor#775
srikrsna-buf merged 4 commits intomainfrom
sk/throw-return-iterables

Conversation

@srikrsna-buf
Copy link
Copy Markdown
Member

@srikrsna-buf srikrsna-buf commented Aug 18, 2023

Call throw and return on response iterables in the node adaptor. This along with #751 ensures that write errors are propagated back to the handler ensuring any cleanup code is run in the handlers.

@srikrsna-buf srikrsna-buf requested a review from timostamm August 18, 2023 07:43
);
isWriteError = false;
for (; !chunk.done; chunk = await it.next()) {
isWriteError = true;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Could do it by using a local try catch but this seemed clearer. I didn't want to rely on node semantics of having a code property.

@srikrsna-buf srikrsna-buf changed the title Call throw and return on handler node: Call throw and return on response iterables Aug 18, 2023
@srikrsna-buf srikrsna-buf changed the title node: Call throw and return on response iterables node: call throw and return on response iterables Aug 18, 2023
@srikrsna-buf srikrsna-buf changed the title node: call throw and return on response iterables call throw and return on response iterables in the node adaptor Aug 18, 2023
@srikrsna-buf srikrsna-buf changed the title call throw and return on response iterables in the node adaptor Call throw and return on response iterables in the node adaptor Aug 18, 2023
@srikrsna-buf srikrsna-buf merged commit fa6330c into main Aug 24, 2023
@srikrsna-buf srikrsna-buf deleted the sk/throw-return-iterables branch August 24, 2023 11:40
@smaye81 smaye81 mentioned this pull request Aug 29, 2023
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.

2 participants