8d2cc4b3 - Add job ticket API and React tracking hook - #197
Draft
TaprootFreak wants to merge 3 commits into
Draft
Conversation
Long-running command endpoints will answer with a ticket instead of holding the request open, so clients need a way to follow one. Adds the Job definition, a JobApi for the status endpoint and, in the React package, a useJob hook plus a context that pools every observed ticket onto a single socket. Polling carries the tracking; the socket only accelerates it. A ticket that loses its socket still reaches its end state, and a transient fetch failure does not end the tracking - only the ten-minute cap does.
Four defects, all in the tracking path. A response for the previous uid could land after a switch and overwrite the new state — worse, cleanup could unsubscribe the *new* ticket, because the refs were shared across effect runs instead of scoped to one. Each run now owns a cancelled flag and its own subscribed uid. Polling and the socket could overtake each other: a late Pending response was able to replace an already reached Complete and revive the timer. Fetches are now single-flight with a settled latch, and nothing is accepted once it is set. The ten-minute cap was only checked between cycles, so a hung request kept the tracking alive indefinitely. It now runs as its own timer from the start. The context held one callback per uid, so two hooks watching the same ticket overwrote each other and the first unsubscribe took the survivor with it. It keeps a set per uid and returns a cleanup that removes only its own. Also: expectedSeconds is typed optional to match how it is handled, socket payloads are validated before use, isOverdue re-evaluates on a deadline timer (its dependence on the client clock is documented, not silently corrected), and the re-export matches its neighbours. The date fields stay typed as Date: every other definition in this package does the same, so changing only this one would add an inconsistency rather than remove one.
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
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.
Why
Command endpoints that need more than a moment are moving to a ticket model: the endpoint answers immediately with a ticket, the work runs in the background, and the client follows the ticket to its result. Clients need a way to do that following, and it should exist once rather than in every consumer.
What
@dfx.swiss/coredefinitions/job.ts—JobStatus,Job<T>,JobUrl, andisJobFinished, so the set of end states is defined in one place instead of being re-listed at every call site.client/JobApi.ts—get<T>(uid)against the status endpoint, registered onDfxApiClientasclient.job.202 Acceptedwith a JSON body arrives at the caller as a success, since that is the response the ticket model relies on.@dfx.swiss/reacthooks/job.hook.ts—useJob(uid)returns the job, a loading flag, anisOverdueflag and an error.contexts/job.context.tsx— pools every observed ticket onto a single socket instead of opening one per ticket.Design decisions worth reviewing
Polling carries the tracking, the socket only accelerates it. A ticket whose socket never connects still reaches its end state. This is deliberate: a push that is treated as the primary path turns every socket problem into a stuck user.
A transient fetch failure does not end the tracking. Only the ten-minute cap does. A network blip is not a broken job, and ending the tracking on one would strand a job that is in fact progressing.
No invented defaults. If
expectedSecondsis missing,isOverduestays false rather than falling back to a made-up threshold.Reconnects are bounded. The context backs off and gives up rather than looping forever.
Additive only — no existing public API changes. Per the contributing guide, this touches source only: no version fields, no changelog, no lockfile.
After the first review round
Ten findings, four of them races in the tracking path. All fixed.
A response for the previous
uidcould land after a switch and overwrite the new state — and worse, the cleanup could unsubscribe the new ticket, because the refs were shared across effect runs instead of scoped to one. Each run now owns its cancelled flag and its own subscribeduid.Polling and the socket could overtake each other. Both wrote every response, so a late
Pendingwas able to replace an already reachedCompleteand revive the timer. Fetches are single-flight now, with a settled latch that is set before timers and subscription are torn down; nothing is accepted afterwards.The ten-minute cap only ran between cycles, so a hung request kept the tracking alive indefinitely. It is now an independent timer started with the effect.
The context held one callback per ticket. Two hooks watching the same
uidoverwrote each other, and the first unsubscribe took the survivor with it. It keeps a set peruidand hands back a cleanup that removes only its own.Smaller ones:
expectedSecondsis typed optional to match how it is actually handled, socket payloads are validated as objects with a stringuidbefore use,isOverduere-evaluates on a deadline timer instead of freezing between responses, and the re-export matches its neighbours.One finding was deliberately not acted on. The date fields are typed
Datewhile the wire carries ISO strings — true, but every other definition in this package does exactly the same, without exception. Changing only this file would have made it the single deviation from a package-wide convention. That belongs in its own decision across the package, not in this PR.