LTRAC-1457: Revalidate via self-fetch instead of a Durable Object queue - #3193
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 201f3bb The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Bundle Size ReportComparing against baseline from No bundle size changes detected. |
Unlighthouse Performance Comparison — VercelComparing PR preview deployment Unlighthouse scores vs production Unlighthouse scores. Summary ScoreAggregate score across all categories as reported by Unlighthouse.
Category Scores
Core Web Vitals
|
1f32dd5 to
7f574b6
Compare
… queue
OpenNext's doQueue routes ISR revalidation through a Durable Object whose
constructor reads env.WORKER_SELF_REFERENCE and throws without it.
queueCache.send() catches that error and only logs, so revalidation would
fail silently rather than surface.
That binding cannot point at this Worker on native hosting. A Cloudflare
service binding resolves against account-level Workers, and a Catalyst
deployment is a script inside a dispatch namespace, which is not
addressable that way. Adding it was attempted and rejected at upload:
400 Bad Request code 10143
Service binding 'WORKER_SELF_REFERENCE' references Worker '…'
which was not found.
Two alternatives were considered and rejected on security grounds, both
because they would expose other tenants:
- A dispatch_namespace binding. Besides the API mismatch — OpenNext calls
.fetch() while that binding exposes .get(name) — .get() accepts any
script in the namespace, letting any deployment invoke any other.
- A service binding to the dispatch router, which is account-level and
would resolve, routing back here by hostname and keeping the Durable
Object queue intact. It would also hand every tenant a handle able to
reach any other tenant's Worker. Public fetches already reach those
endpoints, but only through the Cloudflare edge; an internal handle
bypasses WAF and rate limiting, so it is not equivalent.
Revalidation does not require a binding. It is a HEAD request to the page's
own public URL carrying the build-time preview secret, which is exactly
what the Durable Object issues once it holds the service handle. Issue it
with a plain fetch instead: the subrequest leaves and re-enters through the
dispatch router and arrives at the same Worker, and
global_fetch_strictly_public is already set so it is not short-circuited
internally.
Still wrapped in queueCache, so concurrent stale hits for one path collapse
into a single revalidation. Bounded by a 10s timeout matching the Durable
Object's default, and a non-ok response throws so a failed regeneration is
logged and retried on the next stale hit rather than silently cached as
done.
Lost relative to the Durable Object: retry, max-concurrency, cross-region
dedup, and failed-route backoff. Accepted because the alternative is not
"keep the Durable Object" — it is no revalidation at all.
NEXT_CACHE_DO_QUEUE is left bound. OpenNext's worker template exports all
three DO classes unconditionally, so it still resolves and needs no
migration; it is simply inert.
Latent until ISR is adopted — the build currently produces no routes with a
revalidate window, so the queue is never invoked.
Refs LTRAC-1457
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
7f574b6 to
9726f14
Compare
Correction: the alternatives were narrowed by security, not only by platform limitsThe description above overstates the constraint. Correcting it, and recording the rejected options so they are not re-proposed as oversights. What was proven: a What was not proven: that no service binding can work at all. One would resolve if it pointed at an account-level Worker, and That would have been strictly better than this PR — internal rather than a public round trip, and it keeps the Durable Object queue with its retry, max-concurrency, cross-region dedup and failed-route backoff. It was rejected deliberately. It hands every tenant a handle able to reach any other tenant's Worker. Public fetches already reach those endpoints, so it is not new data access — but only through the Cloudflare edge. An internal handle bypasses WAF, caching and rate limiting, which makes it a usable amplification vector against another tenant. "No tenant holds a handle that can reach another tenant" is a cleaner invariant than re-deriving equivalence each time it comes up. So the full picture is three options, two of which fail on tenant isolation rather than capability:
That is what makes the trade-off here acceptable: the comparison is not "this versus the Durable Object", it is "this versus no revalidation". Also hardened since the first push: the fetch now carries a 10s timeout matching the Durable Object's default, and a non-ok response throws so a failed regeneration is logged by |
Verified end to end on a deployed storeThe concern raised against this approach was that OpenNext's docs say direct-style revalidation "is intended for debugging purposes and is not recommended for use in production. It only works in preview mode (i.e. Deployed 2026-08-26 with a temporary ISR route ( Cycle 1
Cycle 2, after waiting past the revalidate window
Cycle 2 is the more informative one: a stale hit enqueues, No swallowed failures. Zero warn- or error-level log entries across the window. A failing queue would have surfaced Why it works here
Also established
The probe lives on a separate branch ( |
…v var `__NEXT_PREVIEW_MODE_ID` is inlined by Next at build time, so the leading underscores are its naming, not ours, and renaming is not an option. Scoped to the single line rather than the file so the rule keeps applying to the rest of the template. Refs LTRAC-1457 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…change note
The caveat was the last line of a long entry, so a skim read like a caching
improvement — easy to confuse with the regional-cache tag-checking change,
which cost latency rather than saving it.
State up front that no route has a revalidate window today, cite the
prerender manifest, and distinguish fetch-level `next: { revalidate }` from
route-level ISR. Frame the fix as removing a silent-failure trap, since the
`IgnorableError` is dropped under the default log threshold.
Refs LTRAC-1457
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ig comment The header carried the full case against the service binding and the dispatch router, including the 10143 upload error and the cross-tenant reach argument. That is review context, not something a reader of this file needs, and it is already stated at length in the pull request. Keep only what the code cannot convey on its own: that `doQueue` is unusable here, the mechanism the queue replaces it with, and that `global_fetch_strictly_public` is load-bearing — without it the subrequest would be short-circuited and a future reader might reasonably simplify the plain `fetch` away. Point at LTRAC-1457 for the rest. Also separate the queue comment from the `REVALIDATION_TIMEOUT_MS` one, which ran together and read as a single block attached to the constant. Refs LTRAC-1457 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… comment Same treatment as the queue comment above it, applied to the block that came in with LTRAC-1458. The case against enabling purge — the zone-scoped token being readable by merchant application code on a shared tenant zone — is review context, and PR #3183 states it more completely than this comment did, naming the env vars and linking the ignition attempt that was closed for it. Kept the footgun, which is the part a reader of this file needs: that the omission is deliberate, and that declaring `cachePurge` flips OpenNext's defaults on declaration alone regardless of whether it can authenticate. Without that, restoring it looks like a free speed win and silently reintroduces the bug LTRAC-1458 fixed. Dropped the per-hit read cost, which is quantified in the benchmark report on LTRAC-1468 rather than estimated in a comment. Refs LTRAC-1457 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 participants
Linear: LTRAC-1457
Replaces the ignition-side approach, which was tried and proven impossible — see ignition#334 (closed).
What/Why?
OpenNext's
doQueueroutes ISR revalidation through a Durable Object whose constructor readsenv.WORKER_SELF_REFERENCEand throws without it:queueCache.send()catches that and only logs, so revalidation fails silently rather than surfacing.The binding cannot exist on native hosting
This was tested, not assumed. A Cloudflare
servicebinding resolves against account-level Workers, and a Catalyst deployment is a script inside a dispatch namespace, which is not addressable that way. Emitting the binding from ignition was rejected at upload:No value for
servicewould work — it is a platform constraint.The
dispatch_namespacebinding often suggested as the alternative is worse. Beyond the API mismatch (OpenNext calls.fetch()directly; that binding exposes.get(name)),.get()accepts any script in the namespace — binding it into a tenant Worker would let any deployment invoke any other deployment's Worker. Categorically worse than the problem.The fix
Revalidation never needed a binding. It is a
HEADrequest to the page's own public URL carrying the build-time preview secret — exactly what the Durable Object issues once it holds the service handle. This issues it with a plainfetch, which leaves and re-enters throughplatform-dispatch-routerand arrives at the same Worker.global_fetch_strictly_publicis already set, so the subrequest is not short-circuited internally.Still wrapped in
queueCache, so concurrent stale hits for one path still collapse into a single revalidation.Trade-off, stated plainly: the Durable Object's retry and max-concurrency handling is lost. A failed revalidation is retried on the next stale hit rather than by the queue, and revalidations are no longer capped at a concurrency limit. Accepted because the alternative is not "keep the Durable Object" — it is "revalidation does not work at all."
Rollout/Rollback
Latent — no behaviour change for existing stores. The build currently produces zero routes with a revalidate window, so the queue is never invoked. This makes revalidation work for when ISR is adopted rather than fixing an active failure.
NEXT_CACHE_DO_QUEUEis intentionally left bound on both sides. OpenNext's worker template exports all three Durable Object classes unconditionally, so the binding still resolves and no Durable Object migration is needed — deleting the class would have destroyed its storage. It simply goes inert.Rollback is reverting this commit.
Testing
Type-checked against the real build context (
core/.bigcommerce, where the template is compiled), including a deliberate-error probe to confirm the check actually validates and that the queue object satisfies OpenNext'sQueueinterface structurally.Not verifiable through normal traffic, because nothing exercises the queue while there are no ISR routes. Confirming it end-to-end needs a temporary route exporting
revalidateto create a real ISR entry, then observing that a stale hit triggers a regeneration rather than theNo service binding…error. Happy to add that probe if reviewers want it before merge.Refs LTRAC-1457