Repository navigation
fix(runtime): the dispatcher's observability Proxy wraps PUT and PATCH, so those routes carry X-Request-Id and report their 5xx - #24180
Conversation
…H registrations too The Proxy over the dispatcher's IHttpServer instrumented only get, post and delete, so every PUT or PATCH mount answered with no X-Request-Id, left req.requestId unset and never reached errorReporter on a 5xx. It now wraps all five route verbs IHttpServer declares. Three comments that described the three-verb arm are corrected. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QyoWKK9RwUNxBxmevy9Neb
…els on every dispatcher route verb A table over all five verbs on the real plugin (GET/POST/DELETE are the in-file positive control), plus a PUT/PATCH row on the real Hono socket that also pins the transport still counting each request once. The #9650 seam-A measurement keeps its inline copy of the old Proxy, retitled to the past tense. Changeset for @objectstack/runtime (patch). Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QyoWKK9RwUNxBxmevy9Neb
… placeholder Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QyoWKK9RwUNxBxmevy9Neb
…t-patch-observability
📓 Docs Drift CheckThis PR changes 1 package(s): 11 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 5 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 26 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin db63d53c116d6c80466a0d6563a39516ee192e5b && git checkout db63d53c116d6c80466a0d6563a39516ee192e5b
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin cb8756b005a2d77c7c07574dffe4f99b8e91f8d1 2cf6682f87db84646f31e845694f48a3e7b4a714 && git checkout -B drift-repro cb8756b005a2d77c7c07574dffe4f99b8e91f8d1 && git merge --no-ff 2cf6682f87db84646f31e845694f48a3e7b4a714
node scripts/docs-audit/affected-docs.mjs --json cb8756b005a2d77c7c07574dffe4f99b8e91f8d1
|
席位核对 — ACCEPT · PR #24180 at
|
Fixes #24025
Clause-②: no
What was wrong
createDispatcherPluginwraps itsIHttpServerin aProxythat hands each route registration toinstrumentRouteHandler. The trap namedget,postanddeleteonly, so every route the dispatcher mounts throughputorpatchwas registered bare. Those routes answered with noX-Request-Idand leftreq.requestIdunset. A 5xx on them never reachedobservability.errorReporter, and their[5xx]log line carried no method, route or request id.The bare mounts on
origin/mainwere:PUTandPATCH /keys.PUTandPATCHmounts of the three/notificationspaths.PATCH /packages/:id,/enableand/disable.PUT /automation/:name.PUTandPATCH /ai/*wildcards.ai:routes→mountRouteOnServer).This restores a contract that is already published, in two places:
content/docs/protocol/kernel/error-handling.mdx: the id "is attached by the observability instrumentation wrapper", and a deployment that composes it "echoes a valid inboundX-Request-Idand otherwise mints" areq_-prefixed hex id.content/docs/deployment/production-readiness.mdx: "every route the dispatcher mounts now: … EchoesX-Request-Id(honored from caller, or freshly minted). Reports 5xx exceptions to Sentry (or your reporter) with the request id attached."The change
packages/runtime/src/dispatcher-plugin.ts. The Proxy now wraps all five route verbs thatIHttpServerdeclares (get,post,put,delete,patch). The list is typed withsatisfies ReadonlyArray of keyof IHttpServer.use,getRawAppandsetFallbackHandlerstill pass through unchanged.instrumentRouteHandleris unchanged. I read it before deciding: it treats the method only as a metric and reporter label, and nothing in it branches on the verb.afterResponse(the shipped Hono adapter), the wrapper's counter and histogram are already gated off for every verb (emitHttpRequestsTotal/emitHttpRequestDurationMs=!transportCountsRequests). So PUT and PATCH stay at one sample per request; the real-socket row below pins that. On a transport without the seam, the wrapper now counts and times PUT and PATCH under their own method label, as it always did for GET, POST and DELETE./mcp/skillmethod-set note gave the three-verb Proxy as a reason not to mount PUT/PATCH. No route is added there./keysblock and one in the/notificationsblock said their PUT/PATCH mounts are not wrapped.http_requests_totalnever sees auth routes or the REST data API — the two highest-traffic inbound surfaces are outside the only HTTP counter the docs tell operators to monitor #9650 §2 seam-A measurement inhttp-metrics-inbound-coverage.hono.integration.test.tsbuilds its own inline copy of the old three-verb Proxy. Its assertion is unchanged. Its title now reads in the past tense, with a note pointing at the new pin, so it no longer describes today's code.Pins
packages/runtime/src/dispatcher-observability-every-verb.test.ts. It runs the real plugin over a fake transport, with tables over all five verbs. GET, POST and DELETE are the in-file positive control./api/v1/keys: a valid inbound id is echoed and lands onreq.requestId, and one is minted when none is sent./api/v1/ai/*: a route-table handler that throws produces a 500.errorReporterreceives that same error withrequestId,methodandroute. The[5xx]line carries method, path and request id.ai:routes, which reports its 5xx too.afterResponse: one counter sample and one duration sample per request, under the verb's own label.http-metrics-inbound-coverage.hono.integration.test.ts§5. A new case beside the existing GETX-Request-Idcase. Every/api/v1/keysverb over a real Hono socket echoes and mints the header, and the transport still counts and times each request exactly once.Ablation (run once, then restored)
I committed the fix first. Then
scripts/ablation-replace.mjsput the old arm back (prop === 'get' || prop === 'post' || prop === 'delete'), and I ran both pin files. Both import./dispatcher-plugin.jsrelatively, so they read source and need no rebuild.Test Files 2 passed (2),Tests 48 passed (48).Test Files 2 failed (2),Tests 10 failed | 38 passed (48).PUT echoes the inbound id: expected null to be 'probe-put-24025').0136bea52b19equals the HEAD blob, andgit diff HEADis empty.Verification (head
2cf6682f8, after mergingorigin/maincb8756b00)pnpm --filter @objectstack/runtime exec vitest run --project local --maxWorkers=2(the package'stestscript, with workers capped):Test Files 408 passed (408),Tests 6276 passed | 19 skipped (6295), lockVERDICT command-exit 0.pnpm --filter @objectstack/runtime run typecheck: exit 0.check:test-typecheckheld 27 files / 190 errors / 68 pinned signatures, and the new file added no debt.pnpm exec eslint --no-inline-configover the three changed.tsfiles: exit 0, no output.node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstacknamed 62 commands. I ran each with its exit code captured before any pipe.--ranreconciles 62 derived, 62 run, 0 NOT-MEASURED, 0 UNRUN.check-plugin-teardown-shape --self-testneeded its pinned fixture commit fetched on this shallow clone.check:dual-build-cjs-loadsneeded the workspace built (111 require entry points across 69 packages load).check:type-check-debtnever got a turn on the shared lock and was rerun wholly under it (nothing above its ledger).check-changeset-fixed,check:authz-resolver,check:error-code-casing,check:filter-alias-parityandcheck:route-ledger-census.Acceptance notes
RouteDefinition.methodindispatcher-plugin.tsis typed GET, POST, PATCH or DELETE, andmountRouteOnServerhas noputarm. A service that emitted a PUT route onai:routeswould therefore be skipped; only the boot line's mount count would show it. No producer of a PUT route on that hook is named, so nothing is reachable. This is unchanged here and stays outside this card.[5xx]log line but noterrorReporter, on every verb alike. That is consistent with the published "5xx exceptions" / "Unhandled exceptions specifically still reach theerrorReporter" wording, so it is not a finding./mcp/skill,/keysand/notificationsblocks of the same file (H3, extended to the two later comments that said the same thing). It also retitles the one §2 measurement case. Mount helpers, route bodies,plugin-hono-serverandpackages/restare untouched.Generated by Claude Code