Repository navigation
Conversation
size-limit report 📦Compared: 5ef02e1 (PR test merge using develop at 667b2a1) Against: a1162e3 (develop) · base run #65032
|
Optional E2E testsAll optional checks were successful. 4 successful. See the run. Successful checks
|
a357b59 to
e260c45
Compare
e260c45 to
35c5803
Compare
35c5803 to
a2969c3
Compare
cbbbe3b to
335dfa3
Compare
335dfa3 to
126db26
Compare
126db26 to
7198f33
Compare
| const tunnelResult = _INTERNAL_handleLambdaTunnelEvent(event); | ||
| if (tunnelResult) { | ||
| return tunnelResult as Promise<TResult>; | ||
| } |
There was a problem hiding this comment.
Tunnel path skips event-loop freeze
Medium Severity
The new tunnel early-return in wrapHandler never sets callbackWaitsForEmptyEventLoop. That leaves the Lambda default of true, so keep-alive work from forwarding the envelope can delay or block freeze after the tunnel response is already ready.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit a14a93c. Configure here.
| .catch(error => { | ||
| DEBUG_BUILD && debug.error('Sentry tunnel: failed to answer a streaming tunnel request', error); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Feat lacks Lambda tunnel tests
Medium Severity
This feat answers managed tunnel requests on Lambda, but the only new tests cover the _HANDLER redirect takeover. There is no integration or E2E test that sends an HTTP tunnel event through the shim or wrapHandler and checks the response. Adding one would lock in the user-visible behavior.
Additional Locations (1)
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit a14a93c. Configure here.
| }; | ||
| const streamingSymbols = streamingHandler as unknown as Record<symbol, unknown>; | ||
| streamingSymbols[HANDLER_STREAMING_SYMBOL] = handlerSymbols[HANDLER_STREAMING_SYMBOL]; | ||
| streamingSymbols[HANDLER_HIGHWATERMARK_SYMBOL] = handlerSymbols[HANDLER_HIGHWATERMARK_SYMBOL]; |
There was a problem hiding this comment.
Double-casts lack typing comments
Low Severity
New as unknown as double-casts copy streaming symbols onto the tunnel wrapper without a comment on why a safer type is not possible.
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit a14a93c. Configure here.
a14a93c to
3595b95
Compare
Lambda runs no HTTP server, so `serveTunnelIntegration` redirects `_HANDLER` to a shim that answers tunnel events before the user's handler, when `init` runs from a preload. `wrapHandler` in `@sentry/aws-serverless` answers them too. The handler resolution moves to `@sentry/node`, and its loader stays out of the main entry.
3595b95 to
3ba0759
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 5 total unresolved issues (including 3 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3ba0759. Configure here.
| body: await response.text(), | ||
| isBase64Encoded: false, | ||
| }; | ||
| } |
There was a problem hiding this comment.
ALB tunnel responses omit status
High Severity
Tunnel answers for ALB events omit statusDescription, which Application Load Balancer requires on every Lambda response. Those invocations fail at the load balancer with a 502 even when the tunnel itself succeeded, so managed tunnel traffic through ALB never reaches Sentry.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 3ba0759. Configure here.
| return function (this: unknown, event: unknown, context: unknown) { | ||
| return handleTunnelEvent(event) ?? handler.call(this, event, context); | ||
| }; | ||
| } |
There was a problem hiding this comment.
Missing tests for tunnel handling
Medium Severity
This feat has no integration or E2E coverage of answering managed tunnel requests on Lambda. The new tests only check _HANDLER takeover between shims, so event parsing, ALB/API Gateway response shapes, and streaming behavior are unproven.
Additional Locations (2)
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 3ba0759. Configure here.
Adds an AWS handler shim that handles tunnel events before they hit the user handler.
Note for reviewers: if you are lacking context on this PR, check #25997