withSentry wrapper causes stalled request warning with async request handlers in Next.JS #4007
Comments
|
Duplicate of #3852 with possible workaround #3852 (comment) |
lobsterkatie
added a commit
that referenced
this issue
Nov 10, 2021
…warning (#4139) This prevents a false positive warning from nextjs which arises from the interaction of our API route wrapper `withSentry()` with nextjs's native API route handling. In dev, nextjs checks that API route handlers return a response to the client before they resolve, and it throws a warning[1] if this hasn't happened. Meanwhile, in `withSentry()`, we wrap the `res.end()` method to ensure that events are flushed before the request/response lifecycle finishes. As a result, there are cases where the handler resolves before the response is finished, while flushing is still in progress. This triggers the warning mentioned above, but it's a false alarm - the response may not have ended _yet_, but it will end. This suppresses the warning by temporarily marking the response finished, and then restoring it to unfinished before the original `res.end()` is called. The fix here is simple, but the async-i-ness of it all leads to some complex interactions in sequencing between the SDK, nextjs, and Node itself. I've tried to lay it out as clearly as I can in comments. Fixes #4007 Fixes #3852 [1] https://github.com/vercel/next.js/blob/e1464ae5a5061ae83ad015018d4afe41f91978b6/packages/next/server/api-utils.ts#L106-L118
lobsterkatie
added a commit
that referenced
this issue
Feb 8, 2022
…ue (#4516) In #4139, a change was introduced in order to suppress a false positive warning thrown by nextjs, by setting `res.finished` to `true` just long enough for nextjs to check it and decide no warning was needed. In simple cases, it worked just fine, but in some set-ups the "just long enough for nextjs to check it and calm down" turned out to also be "just long enough for other things to check it and get mad." This backs out that change, as it seems it's doing more harm than good. In order to address the original problem (documented in [1] and [2]), a new warning is added, explaining that the false positive is just that. Not as elegant as suppressing the message altogether, but it should tide us over until such time as we're able to try again with a different approach. Fixes #4151. [1] #3852 [2] #4007
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Environment
self-hosted (
onpremisedeployment)Version
^6.12.0
Steps to Reproduce
@sentry/nextjsexport default withSentry(async (req, res) => {res.status(200).end()Expected Result
The route responds and no error is raised.
Actual Result
API resolved without sending a response for /api/<endpoint>, this may result in stalled requests.The text was updated successfully, but these errors were encountered: