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

CE-177 - when the HTTP2Stream is terminated from the remote server handle the close by cleaning up local state and firing an abort error. #639

Draft
wants to merge 4 commits into
base: v4
Choose a base branch
from

Conversation

cleve-fauna
Copy link
Contributor

@cleve-fauna cleve-fauna commented Jun 2, 2022

Notes

Jira Ticket

We have implemented an Http2Adapter in the client which wraps up interaction with Node's underlying HTTP2 library: https://nodejs.org/api/http2.html

This code addresses a bug in the wrapper such that:

  • When nodes serving customer traffic our spun down processes streaming data can hang as our streaming adapter was not alerting the user via an error that the underlying stream was lost.

This fix addresses this issue by handling close events fired off by the remote server such that our adapter's internal state is appropriately updated.

Note that we should only handle these events if two conditions are met:

  1. An error event is not being fired (which we can detect via the rstCode) (see here
  2. The close event is due to the remote server closing the stream; which we can detect via the state.remoteClose property. (see here)

How to test

yarn test and also we setup a Lambda to simulate the server remotely closing the connection by spinning up an AWS Lambda querying Fauna on a regular cadence. The Fauna cluster being queried is having its hosts constantly cycled such that we can reproduce the issue. With this change, the scenario now results in an error to the client, rather than a silent hang.

I can provide a link to the integration test in question.

@cleve-fauna cleve-fauna marked this pull request as draft June 3, 2022 13:44
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