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

Emphasize still talking about sending #3934

Merged
merged 2 commits into from
Jul 28, 2020
Merged

Conversation

MikeBishop
Copy link
Contributor

I totally misread this passage, so let's make it more emphatic which side of the stream we're talking about. Fixes #3933.

@MikeBishop MikeBishop added editorial An issue that does not affect the design of the protocol; does not require consensus. -http labels Jul 21, 2020
Copy link
Member

@martinthomson martinthomson left a comment

Choose a reason for hiding this comment

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

This is the right level of emphasis.

The payload and length of the stream are selected in any manner the sending
implementation chooses. When sending a reserved stream type, the implementation
MAY either terminate the stream cleanly or reset it. When resetting the stream,
the error code H3_NO_ERROR or a reserved error code ({{http-error-codes}})
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
the error code H3_NO_ERROR or a reserved error code ({{http-error-codes}})
either H3_NO_ERROR or a reserved error code ({{http-error-codes}})

H3_NO_ERROR or a reserved error code ({{http-error-codes}}) SHOULD be used.
The payload and length of the stream are selected in any manner the sending
implementation chooses. When sending a reserved stream type, the implementation
MAY either terminate the stream cleanly or reset it. When resetting the stream,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
MAY either terminate the stream cleanly or reset it. When resetting the stream,
MAY either allow the stream to terminate cleanly or reset it. When resetting the stream,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On the sender side, though, either is an explicit action; it's not something you just allow to happen.

Copy link
Contributor

Choose a reason for hiding this comment

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

@martinthomson : Is your point that terminating cleanly is application action where resetting is transport action?

Copy link
Member

Choose a reason for hiding this comment

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

As a receiver you have to let the stream terminate cleanly, the only other action available is a stream reset.

Presumably the sender knows what they are doing, so I wanted to concentrate on the reaction the receiver might have.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The generic advice we give for unknown stream types (which this should be) is to reply with a STOP_SENDING. On the receiver side, you shouldn't be identifying these as grease streams.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is entirely about what the sender does, right?

@MikeBishop MikeBishop merged commit 203377e into master Jul 28, 2020
@MikeBishop MikeBishop deleted the http/grease_error_sending branch July 28, 2020 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-http editorial An issue that does not affect the design of the protocol; does not require consensus.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Recommended behavior for reserved streams
4 participants