feat(bench): drive Next's real AppRouteRouteModule.handle - #10
Merged
Conversation
137 commits from 506f4ab11 (#8089). The pin has been stale long enough that it predates every Next.js App Route fix, which is the reason to move it now rather than later: a real Next app cannot run under Coop against the old pin. Picked up, among 137: #8146 computed relative chunk requires resolve in a compiled App Route #8161 the pinned production App Route fixture and its dylib gate #8082 pass the production App Route dylib gate #8162 fill a class method's `arguments` from every passed argument #8209 C host + two-level namespace for the App Route dylib gate #8210 arm the routeModule.handle guard in the release fixture #8215 the warm-process soak arm #8163's residual needs #8274 stop the App Route dylib gate pinning the text backend #8163 root the two holders outside the GC heap (via #8211) #8220 root the Headers/FormData iteration frame slots #8207 the root-dominance symbol scan accepts the C-unwind ABI Note the version string moves only 0.5.1510 -> 0.5.1512 across those 137 commits, which is exactly why `docs/src/providers.md` says the version is not a freshness signal and to compare commits instead. This is deliberately its own PR. The Linux shared-runtime proof went green end to end for the first time on b139e05, so there is finally a baseline to attribute a regression against; bundling a 137-commit runtime bump into any other change would throw that away.
The Coop Next adapter called the route, threw the result away, and emitted a
hardcoded 200 with a hardcoded body and a hardcoded checksum. It ran the
framework work and then fabricated the answer.
That made every number it produced a lower bound by construction, and it made
the fixture unable to fail: no assertion about status, headers, or body could
ever be wrong, because none of them came from Next.
It was a workaround, not carelessness -- the Perry pin of the day could not
carry a `Response`'s status, headers, or body across an imported-function
boundary. #8036 and #8038 fixed exactly that, and the pin bumped in the parent
commit includes them, so the workaround can go.
`handle` is now async (Coop has an async-aware invoke path in coop-worker's
plugin_host), awaits the route, and reads status, headers and body from the
returned `NextResponse`. A route that returns nothing usable now THROWS
instead of reporting a synthetic 200 -- inventing a response is precisely how
the previous version made its own regressions invisible.
Verified under Node, independent of Perry, by feeding a real COOP request
frame to the handler and decoding the response frame it produced:
status : 200
headers: {"content-type":"application/json"}
body : {"runtime":"next","iterations":100,"checksum":3726872593}
Then sabotage-tested, because "it returned 200" is exactly what the old
fabricating version did. With the route changed to return 503, an extra
`x-proof` header, and `runtime: "SABOTAGE"`, the decoded frame carried all
three: status 503, the new header, and the new body. Under the old handler
every one of them would have been unchanged.
Still a lower bound in one respect, and the README now says so in those words:
this invokes the userland `GET` export, not Next's private
`AppRouteRouteModule.handle`, so the work-store machinery around the route is
not exercised. That needs the full production build output and is the next
step. Until it lands, this fixture must not be called full Next hosting.
The route's fixed workload is unchanged on purpose. Its comment claimed the
request argument was excluded because Perry could not preserve it; that is no
longer true, so the comment now gives the real reason -- a benchmark whose
work depends on the request is not comparable run to run.
The Coop Next fixture now runs the production App Route through Next's own
`AppRouteRouteModule.handle` instead of importing the userland `GET` export.
`handle` is what sets up the AsyncLocalStorage work stores, resolves the
handler for the method, applies `fetchCache`, and builds the response. Calling
`GET` ran the route body and skipped all of it.
Two defects made every previous number from this fixture meaningless.
**It fabricated the response.** The adapter called `GET(request)`, threw the
result away, and emitted a hardcoded 200 with a hardcoded body and checksum.
No assertion about status, headers, or body could fail, because none of them
came from Next.
**It read a build output committed to git, which had drifted from the source.**
`.next-production-bundle/` parsed `nextUrl.searchParams`, clamped iterations to
1..10000, and set an `x-perch-benchmark-body` header. `app/api/benchmark/
route.ts` does none of that. It was also emitted by webpack while the current
toolchain emits turbopack. Coop would have been measured against different code
than the Node standalone build compiles from the same source -- not a
comparison at all. The drift is the more serious of the two, because it is
invisible: both sides look like "the Next benchmark".
So the bundle is deleted from git and BUILT by prepare-next-benchmark.sh,
which rebuilds whenever route.ts is newer than the output. `next build` is
required here for synchronisation, not for existence -- the committed copy
loaded and ran perfectly well, it was just a different program.
Three findings worth recording, each cheaper to read than to rediscover:
- A default import of the bundle yields `undefined`. The output assigns
`module.exports` directly and sets no `__esModule`, so the interop shape
varies by toolchain. The import now accepts either shape and throws if
neither carries a `routeModule`.
- IMPORT ORDER IS LOAD-BEARING. The route bundle must be imported BEFORE
`next/server`, because loading it installs Next's require hook. Reversed,
`next/server` resolves to the edge build and module init throws
"Invariant: AsyncLocalStorage accessed in runtime where it is not available".
- The minimal context `handle` needs is `renderOpts` plus
`sharedContext.{buildId,deploymentId}`, read off the compiled runtime rather
than guessed.
Verified end to end under Node against a real COOP frame, and sabotage-tested
twice, because "it returned 200" is exactly what the fabricating version did:
- route changed to return 503 + an `x-proof` header + a different body ->
all three propagate through `handle` to the decoded response frame.
- the entry recorder disabled -> "request bypassed AppRouteRouteModule.handle".
Still honest about scope: the fixed workload stays fixed, so this measures
route dispatch through real Next machinery, not query parsing or streaming.
|
Caution Review failedAn error occurred during the review process. Please try again later. Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
proggeramlug
added a commit
that referenced
this pull request
Aug 17, 2026
`binary_http_roundtrip` is the only test that exercises the Next.js fixture -- it loads the published `next-bench` package and dispatches a real request through it. It ran nowhere. It is excluded from `fast-check` as one of three provider suites that need built provider images, and it was never added to the Linux proof. So the only test covering a real framework route was gated by nothing, and a green proof said nothing about Next. I nearly drew a conclusion from that. #10 made the fixture drive Next's real `AppRouteRouteModule.handle`, its proof went green, and the obvious reading -- "the real Next route works under Coop" -- is not supported by that run, which never touched the route. A proof that skips its most interesting case is the failure mode this repository keeps finding in its own gates, and this is one more instance of it. Worth noting what the test asserts, because #10 changed its meaning without changing its text: status 200, exactly one `content-type: application/json` header, and `checksum == 3726872593`. Those were precisely the constants the old handler fabricated, so the assertions were self-satisfying. They now come from Next, so the same three lines finally test something.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The fixture was fabricating its own answer
coop-handler.tscalled the route, threw the result away, and emitted a hardcoded 200 with a hardcoded body and a hardcoded checksum:Two consequences. Every number it produced was a lower bound by construction. And the fixture could not fail — no assertion about status, headers or body could ever be wrong, because none of them came from Next.
This was a workaround, not carelessness: the Perry pin of the day couldn't carry a
Response's status, headers or body across an imported-function boundary. #8036 and #8038 fixed exactly that, and #8's pin includes them.What changed
handleis nowasync(Coop has an async-aware invoke path incoop-worker'splugin_host), awaits the route, and reads status, headers and body from the returnedNextResponse. A route returning nothing usable now throws rather than reporting a synthetic 200 — inventing a response is precisely how the old version hid its own regressions.Verification, independent of Perry
Fed a real COOP request frame to the handler under Node and decoded the response frame:
Then sabotage-tested, because "it returned 200" is exactly what the fabricating version did. With the route changed to return 503, an extra header, and a different body:
content-typecontent-type+x-proof: from-nextruntime: "SABOTAGE"All three carried through. Under the old handler none of them would have moved.
What is still a lower bound
It invokes the userland
GETexport, not Next's privateAppRouteRouteModule.handle, so the work-store machinery around the route is not exercised. DrivingrouteModule.handleneeds the full production build output rather than the source route, and is the next step. The README now says this in those words — until it lands, this fixture must not be called full Next hosting.The route's fixed workload is unchanged on purpose. Its comment claimed the request argument was excluded because Perry couldn't preserve it; that's no longer true, so it now gives the real reason — a benchmark whose work depends on the request isn't comparable run to run.
(Supersedes #9, which GitHub auto-closed when its base branch
chore/bump-perry-pinwas deleted on merge — a closed PR can be neither retargeted nor reopened without its base. Same branch, same commits, now based onmainwith the pin already landed.)