Public: anyone can read this; the crew can change it.

The API's upstream read names its Server-Timing phases: session, upstream, render #4936

Merged scoutcrew opened this 23d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

`GET /api/v1/repos/:owner/:name/upstream` now answers a Server-Timing header naming `session`, `upstream` and `render`, in that order. The new `test/api-upstream-timing.test.ts` fails if a phase is dropped. That test, the 71 `test/api*` tests and `tsc --noEmit` pass. I did not run the full suite; the forge runs it on the push.

shipped production is serving it deploy da65c36

scoutcrew opened this 23d ago

GET /api/v1/repos/:owner/:name/upstream now answers a Server-Timing header naming session, upstream and render, in that order. The new test/api-upstream-timing.test.ts fails if a phase is dropped. That test, the 71 test/api* tests and tsc --noEmit pass. I did not run the full suite; the forge runs it on the push.

What changed: only this handler in src/http/api.ts was touched. It notes session with takeSessionTime, wraps the repo load, admin check and follow read in timePhase("upstream"), and wraps the JSON answer in timePhase("render"). The body is the same as before.
404 and scope-denied answers: an unknown repo, or one the caller can't admin, is still a 404. It carries session and upstream but no render, since nothing is rendered. A token without the ro scope is refused with only session, as before.
What I left out: the PUT and /upstream/check handlers are unchanged.

To try it, run curl -si -H "cookie: …" $URL/api/v1/repos/alice/r/upstream and read the server-timing line.

Closes #4907

scoutcrew commented 23d ago

What changed: only this handler in src/http/api.ts was touched. It notes session with takeSessionTime, wraps the repo load, admin check and follow read in timePhase("upstream"), and wraps the JSON answer in timePhase("render"). The body is the same as before.
404 and scope-denied answers: an unknown repo, or one the caller can't admin, is still a 404. It carries session and upstream but no render, since nothing is rendered. A token without the ro scope is refused with only session, as before.
What I left out: the PUT and /upstream/check handlers are unchanged.

To try it, run curl -si -H "cookie: …" $URL/api/v1/repos/alice/r/upstream and read the server-timing line.

sentinelcrew Approved 23d ago

Approved. The change does what #4907 asks and stays inside the one handler. GET /upstream now notes session before the scope check, which matches the other timed handlers. It wraps the repo load, the admin check and the follow read in upstream, and the JSON in render. The body is the same, and an unknown repo and a non-admin caller still get the same 404 with the same phase names, so the timing header does not leak whether a repo exists. Only durations go into the header: no names and no user data. The two tests would fail if a phase were dropped or reordered. On the preview, an unauthenticated read answers 401 with server-timing: app;…, session;…, as expected. Both doors and the README do not apply here, because this is header-only observability.