quic: another round of refactorings #34247
Closed
Conversation
This file contains 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
The argument will always be a boolean already
nodejs-github-bot
added
c++
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.
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.
This comment has been minimized.
jasnell
force-pushed
the
quic-cleanups-4
branch
2 times, most recently
from
c767d37
to
f044069
Compare
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
|
Ping @nodejs/quic... this is ready for review |
|
Both CI's are good. |
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
jasnell
added a commit
that referenced
this pull request
Jul 9, 2020
PR-URL: #34247 Reviewed-By: Anna Henningsen <[email protected]>
jasnell
added a commit
that referenced
this pull request
Jul 9, 2020
PR-URL: #34247 Reviewed-By: Anna Henningsen <[email protected]>
jasnell
added a commit
that referenced
this pull request
Jul 9, 2020
PR-URL: #34247 Reviewed-By: Anna Henningsen <[email protected]>
jasnell
added a commit
that referenced
this pull request
Jul 9, 2020
PR-URL: #34247 Reviewed-By: Anna Henningsen <[email protected]>
jasnell
pushed a commit
that referenced
this pull request
Jul 9, 2020
0f97d60 accidentally removed this. Refs: #34186 PR-URL: #34247 Reviewed-By: James M Snell <[email protected]>
jasnell
added a commit
that referenced
this pull request
Jul 9, 2020
PR-URL: #34247 Reviewed-By: Anna Henningsen <[email protected]>
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 <[email protected]>
jasnell
added a commit
that referenced
this pull request
Jul 9, 2020
PR-URL: #34247 Reviewed-By: Anna Henningsen <[email protected]>
jasnell
added a commit
that referenced
this pull request
Jul 9, 2020
PR-URL: #34247 Reviewed-By: Anna Henningsen <[email protected]>
jasnell
added a commit
that referenced
this pull request
Jul 9, 2020
PR-URL: #34247 Reviewed-By: Anna Henningsen <[email protected]>
jasnell
added a commit
that referenced
this pull request
Jul 9, 2020
PR-URL: #34247 Reviewed-By: Anna Henningsen <[email protected]>
jasnell
added a commit
that referenced
this pull request
Jul 9, 2020
PR-URL: #34247 Reviewed-By: Anna Henningsen <[email protected]>
|
Landed in e8f5745...26493c0 |
This was referenced Jul 10, 2020
|
Marked as "backport-requested" because I do not believe quic is on 14.x yet. Should we change the label to "dont-land"? |
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
make -j4 test(UNIX), orvcbuild test(Windows) passes