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

Better manage stream channel activation. #124

Merged
merged 1 commit into from
May 17, 2019

Conversation

Lukasa
Copy link
Contributor

@Lukasa Lukasa commented May 17, 2019

Motivation:

Our existing model for working out whether a stream channel is active is
a bit too ad-hoc. We "activate" a stream channel once the initializer
has completed so long as the parent channel is active. This approach
is basically reasonable, but it makes it a bit hard to handle stream
closure.

Specifically, in the client case a stream channel may be "active" but
have never sent a frame. In this case we can (and must) close the stream
without sending a frame if the user requests closure. This means we
need to know whether or not the stream is actually open from the
perspective of the parent channel.

Modifications:

  • Added new notifications about whether the stream is active on the
    network
  • Enhanced the stream channel to be more clear about its state.
  • Wrote some new tests.
  • Fixed old tests that needed updating.

Result:

HTTP2StreamChannel will be more resilient to weird network behaviour.

Motivation:

Our existing model for working out whether a stream channel is active is
a bit too ad-hoc. We "activate" a stream channel once the initializer
has completed so long as the parent channel is active. This approach
is basically reasonable, but it makes it a bit hard to handle stream
closure.

Specifically, in the client case a stream channel may be "active" but
have never sent a frame. In this case we can (and must) close the stream
without sending a frame if the user requests closure. This means we
need to know whether or not the stream is actually open from the
perspective of the parent channel.

Modifications:

- Added new notifications about whether the stream is active on the
    network
- Enhanced the stream channel to be more clear about its state.
- Wrote some new tests.
- Fixed old tests that needed updating.

Result:

HTTP2StreamChannel will be more resilient to weird network behaviour.
@Lukasa Lukasa added the 🔨 semver/patch No public API change. label May 17, 2019
@Lukasa Lukasa added this to the 1.2.1 milestone May 17, 2019
@Lukasa Lukasa requested a review from weissi May 17, 2019 13:06
Copy link
Member

@weissi weissi left a comment

Choose a reason for hiding this comment

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

thanks, looks great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants