quic: another round of refactorings - #34247
Closed
jasnell wants to merge 16 commits into
Closed
Conversation
Copy link
Copy Markdown
Member
Sorry, something went wrong.
The argument will always be a boolean already
Errors thrown within the session event handler will be handled by destroying the session (allowing a proper connection close to be sent to the client peer). They will not crash the parent QuicSocket by default. Instead, a `'sessionError'` event will be emitted, allowing the error to be logged or handled.
Also, change setServerBusy into a setter
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
0f97d60 accidentally removed this. Refs: nodejs#34186
This comment has been minimized.
jasnell
force-pushed
the
quic-cleanups-4
branch
2 times, most recently
from
c767d37 to
f044069
Compare
July 8, 2020 16:06
Some of the flags were no longer being used. Switched to use an AliasedStruct for shared state to avoid extraneous expensive JS=>C++ calls. Removed unused QuicSocket option
jasnell
force-pushed
the
quic-cleanups-4
branch
2 times, most recently
from
e5cea72 to
2a61277
Compare
July 8, 2020 17:05
jasnell
marked this pull request as ready for review
July 8, 2020 17:07
Copy link
Copy Markdown
Member
Author
|
Ping @nodejs/quic... this is ready for review |
Sorry, something went wrong.
Copy link
Copy Markdown
Member
Author
|
Both CI's are good. |
Sorry, something went wrong.
addaleax
approved these changes
Jul 8, 2020
20 tasks
jasnell
added a commit
that referenced
this pull request
Jul 9, 2020
PR-URL: #34247 Reviewed-By: Anna Henningsen <anna@addaleax.net>
jasnell
added a commit
that referenced
this pull request
Jul 9, 2020
The argument will always be a boolean already PR-URL: #34247 Reviewed-By: Anna Henningsen <anna@addaleax.net>
jasnell
added a commit
that referenced
this pull request
Jul 9, 2020
Errors thrown within the session event handler will be handled by destroying the session (allowing a proper connection close to be sent to the client peer). They will not crash the parent QuicSocket by default. Instead, a `'sessionError'` event will be emitted, allowing the error to be logged or handled. PR-URL: #34247 Reviewed-By: Anna Henningsen <anna@addaleax.net>
jasnell
added a commit
that referenced
this pull request
Jul 9, 2020
Also, change setServerBusy into a setter PR-URL: #34247 Reviewed-By: Anna Henningsen <anna@addaleax.net>
jasnell
added a commit
that referenced
this pull request
Jul 9, 2020
PR-URL: #34247 Reviewed-By: Anna Henningsen <anna@addaleax.net>
jasnell
added a commit
that referenced
this pull request
Jul 9, 2020
PR-URL: #34247 Reviewed-By: Anna Henningsen <anna@addaleax.net>
jasnell
added a commit
that referenced
this pull request
Jul 9, 2020
PR-URL: #34247 Reviewed-By: Anna Henningsen <anna@addaleax.net>
jasnell
added a commit
that referenced
this pull request
Jul 9, 2020
PR-URL: #34247 Reviewed-By: Anna Henningsen <anna@addaleax.net>
jasnell
added a commit
that referenced
this pull request
Jul 9, 2020
PR-URL: #34247 Reviewed-By: Anna Henningsen <anna@addaleax.net>
jasnell
added a commit
that referenced
this pull request
Jul 9, 2020
Some of the flags were no longer being used. Switched to use an AliasedStruct for shared state to avoid extraneous expensive JS=>C++ calls. Removed unused QuicSocket option PR-URL: #34247 Reviewed-By: Anna Henningsen <anna@addaleax.net>
jasnell
added a commit
that referenced
this pull request
Jul 9, 2020
PR-URL: #34247 Reviewed-By: Anna Henningsen <anna@addaleax.net>
jasnell
added a commit
that referenced
this pull request
Jul 9, 2020
PR-URL: #34247 Reviewed-By: Anna Henningsen <anna@addaleax.net>
jasnell
added a commit
that referenced
this pull request
Jul 9, 2020
PR-URL: #34247 Reviewed-By: Anna Henningsen <anna@addaleax.net>
jasnell
added a commit
that referenced
this pull request
Jul 9, 2020
PR-URL: #34247 Reviewed-By: Anna Henningsen <anna@addaleax.net>
jasnell
added a commit
that referenced
this pull request
Jul 9, 2020
PR-URL: #34247 Reviewed-By: Anna Henningsen <anna@addaleax.net>
This was referenced Jul 10, 2020
Copy link
Copy Markdown
Contributor
|
Marked as "backport-requested" because I do not believe quic is on 14.x yet. Should we change the label to "dont-land"? |
Sorry, something went wrong.
This file contains hidden or 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
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.
This is a WIP, more will be added:
Commits:
quic: additional minor cleanups in node_quic_session.h
Minor miscellaneous cleanups
quic: remove unnecessary bool conversion
Self-explanatory
quic: handle errors thrown / rejections in the session event
quic: refactor/improve error handling for busy event
quic: add tests confirming error handling for QuicSocket close event
quic: refactor/improve QuicSocket ready event handling
quic: refactor/improve QuicSocket ready event handling
Starting to improve/refactor/verify error handling on events
quic: use Number() instead of bigint for QuicSocket stats
quic: unref timers again
Fix a bug
quic: use getter/setting for stateless reset toggle
Improve API ergonomics
quic: cleanup QuicSocketFlags, used shared state struct
Use AliasedStruct for QuicSocket shared state, cleanup no longer used state flags
quic: proper custom inspect for QuicEndpoint/QuicSocket/QuicSession/QuicStream
Self explanatory
Checklist