Repository navigation
feat(deno): Record request sessions for Deno.serve - #25496
Conversation
isaacs
added this pull request to stack #25494
October 19, 2026 02:07
isaacs
requested review from
JPeer264 and
mydea
and removed request for
a team
October 19, 2026 02:07
Member
Author
|
Bugbot run |
`denoHttpIntegration` records a request session for each `node:http` request, but `denoServeIntegration` recorded none. Most Deno apps use `Deno.serve`, so release health showed no request sessions and no crash-free rate for them. The shared helper `recordRequestSession` waits for the `close` event of a `node:http` response, which `Deno.serve` does not have. Core now exports `startRequestSession`, which starts the session and returns an `end()` function. `recordRequestSession` keeps its signature and calls `end()` from the `close` event, so Node does not change. The helper now defaults `sessionFlushingDelayMS` to 60 seconds. Before, only core's `node:http` subscription code set that default, and a caller that left the option out got a timer that fired at once. `denoServeIntegration` accepts `sessions` (default `true`) and `sessionFlushingDelayMS`, as `denoHttpIntegration` does. The session ends when the response finishes. For a streamed response, that is when the stream finishes. When the handler throws, the session ends after the SDK captures the error, so it counts as crashed. Sessions do not depend on tracing, so they are also recorded when the SDK skips the span. To match Node, OPTIONS and HEAD requests also record sessions. Fixes #25495 Fixes JS-3956 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
isaacs
force-pushed
the
isaacschlueter/js-3956-deno-denoserve-does-not-record-request-sessions
branch
from
October 19, 2026 02:14
e420c76 to
23896e3
Compare
Contributor
size-limit report 📦
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 23896e3. Configure here.
| requestIsolationScope: isolationScope, | ||
| sessionFlushingDelayMS: integration.sessionFlushingDelayMS, | ||
| }) | ||
| : undefined; |
There was a problem hiding this comment.
Session timer blocks Deno process exit
Medium Severity
denoServeIntegration now ends request sessions through startRequestSession, which schedules a 60s aggregate setTimeout. safeUnref only calls unref() when the timer is an object, but Deno’s setTimeout returns a numeric id, so the timer stays referenced. After a Deno.serve request, a CLI or short-lived Deno process can sit in the event loop until that delay elapses, even when no release is set and sendSession will drop the aggregate.
Additional Locations (1)
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 23896e3. Configure here.
This branch has not been deployed
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
1 participant
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.
denoHttpIntegrationrecords a request session for eachnode:httprequest, butdenoServeIntegrationrecorded none. Most Deno apps useDeno.serve, so release health showed no request sessions and no crash-free rate for them.The shared helper
recordRequestSessionwaits for thecloseevent of anode:httpresponse, whichDeno.servedoes not have. Core now exportsstartRequestSession, which starts the session and returns anend()function.recordRequestSessionkeeps its signature and callsend()from thecloseevent, so Node does not change. The helper now defaultssessionFlushingDelayMSto 60 seconds. Before, only core'snode:httpsubscription code set that default, and a caller that left the option out got a timer that fired at once.denoServeIntegrationacceptssessions(defaulttrue) andsessionFlushingDelayMS, asdenoHttpIntegrationdoes. The session ends when the response finishes. For a streamed response, that is when the stream finishes. When the handler throws, the session ends after the SDK captures the error, so it counts as crashed. Sessions do not depend on tracing, so they are also recorded when the SDK skips the span. To match Node, OPTIONS and HEAD requests also record sessions.Fixes #25495
Fixes JS-3956