Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(nuxt/nitro): Flush with waitUntil after response #13895

Closed
wants to merge 3 commits into from

Conversation

s1gr1d
Copy link
Member

@s1gr1d s1gr1d commented Oct 7, 2024

When deploying a Nuxt page to Vercel, Sentry can capture events but the server instance is closed too early. With waitUntil, Vercel can wait for Sentry to flush all events.

@s1gr1d s1gr1d requested review from lforst and Lms24 October 7, 2024 14:36
@@ -29,10 +29,16 @@ export default defineNitroPlugin(nitroApp => {
captureContext: { contexts: { nuxt: structuredContext } },
mechanism: { handled: false },
});

vercelWaitUntilAndFlush();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we have to await waitUntil or am I misunderstanding how this should work?

Copy link
Member

@lforst lforst Oct 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

waitUntil is a worker API that simply tells the runtime not to freeze execution until the passed promise has resolved. So no need to await it. (awaiting would probably even be bad because it would block the response?)

@s1gr1d
Copy link
Member Author

s1gr1d commented Oct 15, 2024

Can be closed in favor of this PR: #13986

@s1gr1d s1gr1d closed this Oct 15, 2024
s1gr1d added a commit that referenced this pull request Oct 17, 2024
With
[waitUntil](https://vercel.com/docs/functions/functions-api-reference#waituntil)
the lambda execution continues until all async tasks (like sending data
to Sentry) are done.

Timing-wise it should work like this: `span.end()` -> `waitUntil()` ->
Nitro/Node `response.end()`

The problem in [this
PR](#13895) was that
the Nitro hook `afterResponse` is called to late (after
`response.end()`), so `waitUntil()` could not be added to this hook.

---

Just for reference how this is done in Nitro (and h3, the underlying
http framework):

1. The Nitro `afterResponse` hook is called in `onAfterResponse`

https://github.com/unjs/nitro/blob/359af68d2b3d51d740cf869d0f13aec0c5e9f565/src/runtime/internal/app.ts#L71-L77

2. h3 `onAfterResponse` is called after the Node response was sent (and
`onBeforeResponse` is called too early for calling `waitUntil`, as the
span just starts at this point):

https://github.com/unjs/h3/blob/7324eeec854eecc37422074ef9f2aec8a5e4a816/src/adapters/node/index.ts#L38-L47

- `endNodeResponse` calls `response.end()`:
https://github.com/unjs/h3/blob/7324eeec854eecc37422074ef9f2aec8a5e4a816/src/adapters/node/internal/utils.ts#L58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants