fix: guard ctx.create() against ended context after client disconnect#109
fix: guard ctx.create() against ended context after client disconnect#109melikhov-dev merged 2 commits intomainfrom
Conversation
Add isEnded() checks in wrapMiddleware and wrapRouteHandler to prevent crashes when middleware or route handlers run after the client disconnects and the original context is already ended.
melikhov-dev
force-pushed
the
fix/guard-ctx-create-after-disconnect
branch
2 times, most recently
from
dd837ca to
299dedb
Compare
src/tests/context-lifecycle.test.ts
Outdated
| const nodekit = new NodeKit(); | ||
| const app = new ExpressKit(nodekit, { | ||
| 'GET /test': { | ||
| beforeAuth: [slowMiddleware], |
There was a problem hiding this comment.
suggestion(blocking): Two tests look almost the same. Looks like you should add another middleware here to test this case
| "@commitlint/config-conventional": "^17.7.0", | ||
| "@gravity-ui/eslint-config": "^3.2.0", | ||
| "@gravity-ui/nodekit": "^2.7.0", | ||
| "@gravity-ui/nodekit": "^2.10.1", |
There was a problem hiding this comment.
issue(blocking): You should up nodekit in peer dependencies because you use isEnded function
There was a problem hiding this comment.
Maybe it would be better to add some fallback to not make a major release because of upgrading peer deps
There was a problem hiding this comment.
issue(blocking): You should up nodekit in peer dependencies because you use
isEndedfunction
Switched to reqCtx.abortSignal.aborted for backward compatibility.
There was a problem hiding this comment.
issue(blocking): You should up nodekit in peer dependencies because you use
isEndedfunctionSwitched to
reqCtx.abortSignal.abortedfor backward compatibility.
But abortSignal was added in the 2.5.0 version of nodekit, and we support ^1.6.0 || ^2.0.0 in expresskit.
There was a problem hiding this comment.
But
abortSignalwas added in the 2.5.0 version of nodekit, and we support^1.6.0 || ^2.0.0in expresskit.
Good catch! Added optional chaining (abortSignal?.aborted) so it won't break on nodekit < 2.5.0. The guard simply won't activate, preserving the old behavior.
melikhov-dev
force-pushed
the
fix/guard-ctx-create-after-disconnect
branch
from
299dedb to
cfdb360
Compare
Required for isEnded() method on AppContext type.
melikhov-dev
force-pushed
the
fix/guard-ctx-create-after-disconnect
branch
from
cfdb360 to
ac08d52
Compare
2 participants
Closes #108
Summary
isEnded()guards inwrapMiddlewareandwrapRouteHandler(src/router.ts) to preventctx.create()from throwing when middleware or route handler runs after client disconnects and the parent context is already endedwrapMiddleware: skip child context creation and callnext()earlywrapRouteHandler: skip handler execution entirelyTest plan
context-lifecycle.test.tsthat simulate real client disconnect viasocket.destroy()Error: Trying to create child context from already ended context(unhandled rejection)