Skip to content

fix: use waitUntil if available to await the fetch cache set promise #75971

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

Open
wants to merge 1 commit into
base: canary
Choose a base branch
from

Conversation

dario-piotrowicz
Copy link

@dario-piotrowicz dario-piotrowicz commented Feb 12, 2025

The promise created for updating the incremental cache on successful fetches is not used, returned nor awaited in any way. I imagine that this can be fine in environments such as Node.js servers where the dangling promise will eventually run, in other environments however the promise might be cancelled causing the cache update not to happen, so it would be appropriate, when possible that the promise is awaited using waitUntil (for more context, I've encountered this issue in the OpenNext project).

cc. @vicb, @conico974

@ijjk
Copy link
Member

ijjk commented Feb 12, 2025

Allow CI Workflow Run

  • approve CI run for commit: b546102

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

@@ -727,6 +728,15 @@ export function createPatchedFetcher(
)
.finally(handleUnlock)

const waitUntil = getBuiltinRequestContext()?.waitUntil
Copy link
Member

Choose a reason for hiding this comment

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

Instead of pulling in the RequestContext here we could push this onto the pendingRevalidates array which already gets wired to the available waitUntil.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants