fix(@libp2p/webtransport): be more thorough about closing sessions #1969
Conversation
Refactors session closing to happen in one function and call that function when the session has closed or failed to init. Doesn't quite solve the "Too many pending WebTransport Sessions" problem but does slow it down a little bit. Refs: #1896
| })) | ||
| }) | ||
|
|
||
| cleanUpWTSession = (closeInfo?: WebTransportCloseInfo) => { |
There was a problem hiding this comment.
why define this function inside dial instead of making it a method on the webtransportTransport class?
There was a problem hiding this comment.
It needed access to a lot of scoped variables, and it's passed to the stream muxer so it got a bit fiddly.
| maConn = { | ||
| close: async (options?: AbortOptions) => { | ||
| log('Closing webtransport') | ||
| cleanUpWTSession() | ||
| }, | ||
| abort: (err: Error) => { | ||
| log('aborting webtransport due to passed err', err) | ||
| cleanUpWTSession({ | ||
| closeCode: 0, | ||
| reason: err.message | ||
| }) | ||
| }, | ||
| remoteAddr: ma, | ||
| timeline: { | ||
| open: Date.now() | ||
| }, | ||
| // This connection is never used directly since webtransport supports native streams. | ||
| ...inertDuplex() | ||
| } |
There was a problem hiding this comment.
if we make this a property on the class we can split out this dial function a bit
| // this promise resolves/throws when the session is closed or failed to init | ||
| wt.closed | ||
| .then(async () => { | ||
| await maConn?.close() | ||
| }) | ||
| .catch((err: Error) => { | ||
| log.error('error on remote wt session close', err) | ||
| maConn?.abort(err) | ||
| }) | ||
| .finally(() => { | ||
| // if we never got as far as creating the maConn, just clean up the session | ||
| if (maConn == null) { | ||
| cleanUpWTSession() | ||
| } | ||
| }) |
There was a problem hiding this comment.
I believe this should cover the case where await wt.ready throws, but from my testing, I'm not confident the .closed catch always handles errors thrown from wt.ready.
There was a problem hiding this comment.
I think this is an oddity of the clean up procedure - .ready and .closed are treated separately.
Refactors session closing to happen in one function and call that function when the session has closed or failed to init.
Doesn't quite solve the "Too many pending WebTransport Sessions" problem but does slow it down a little bit.
Refs: #1896