fix(sdk): one HTTP loader and one retry policy for every file of a model (#765) - #775
Merged
Merged
Conversation
…4 and carries the credentials asked (#765)
… one HTTP loader (#765)
…when their model leaves (#765)
… SDK guide, the engine notes and the API reference (#765)
…sed body let go, the page cache asks no 4xx twice (#765)
… page cache, the retry worded as the reads do it (#765)
…eamer asks no 4xx twice, the physics reads let go with their model (#765)
…d file without soft bodies opens, the code's meaning says when a read is asked again (#765)
…a number of attempts, no overloads (#765)
….error names it, tests for a 5xx image, a refused lights.json and a plain PageSource failure (#765)
… physics error doc within 200 lines (#765)
…a 5xx, after the wait its Retry-After asks (#765)
… and loop over their cases (#765)
…ked reads leave the default to the loader, the guide says which retries wait (#765)
…once, and a test aborts that wait (#765)
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.
Closes #765
What changed
Every scene resource the engine fetches now goes through one loader,
checked(packages/sdk-browser/src/cluster/pages.ts). It takes the most requests it makes (attempts, 2 by default), andoptionalFilereads a file that may be absent:Retry-Afterasks (seconds or an HTTP date); any other 4xx never (CTO decision C).optionalFile), a 404 or a 403 answersnull(CTO decision A). A required read fails on either.ONE_REQUEST(1).Two helpers keep that rule in one place:
refusedStatus(error)andretriableError(error). The GPU page cache, the page streamer and the physics tiles use them.credentialsis not an option. Its only use,same-originfor images, is already fetch's default, so the issue's "gainscredentials" has no user and the field was removed.Behaviour changes, by call site:
gpu/page/pages.tshttpPageSourceRESOURCE_HTTP_ERRORwithdetails.urlanddetails.status, instead ofError('PAGE_HTTP_<status>').details.status. A hostPageSourcethrowing the old undocumentedPAGE_HTTP_<n>now reportsstatus: null.gpu/page/load.ts) no longer retries a 4xx: a missing page is asked once instead of twice. A 5xx or network failure is still retried once.streaming/fetch.ts, the page streamer (decision B)checkedwith one request per attempt.after one attempt.host/prepared/images.tsRESOURCE_HTTP_ERRORinstead of a plainError.lighting/importedLights.tsRESOURCE_HTTP_ERROR; before, it quietly meant no lights.physics/tilePlace.tscookedPhysics, thephysics.jsonreadRESOURCE_HTTP_ERRORonworld.physics.error. Before, a 401 or 500 was silent and a network failure raised a rawTypeError.cookedBytes,ONE_REQUEST)RESOURCE_HTTP_ERRORnaming the address, notPHYSICS_FAILED.AbortController. When the model leaves, the reads of itsphysics.jsonand tiles still in flight are aborted, and that is not reported as a failure. Before, they downloaded in full and the result was dropped.physics/cookedSoft.ts)RESOURCE_HTTP_ERROR, retried once after a 5xx, instead ofPHYSICS_FAILEDwith no retry.physics.json, so an aborted read is never reused.?? []).Other
fetch(calls inpackages/were not moved:page/decode/geometryPageWasm.ts:76andphysics/physicsWorker.ts:115load the engine's own wasm, not scene resources.cluster/pages.tsthere would pull the decode pool into the physics worker, and for geometryPageWasm it would create an import cycle.Documentation:
docs/SDK.mdunder "Files over HTTP". The lights and physics sections anddocs/ENGINE.mdlink to it.RESOURCE_HTTP_ERRORmeaning ("asked again first when the failure may pass") is updated inscripts/api-reference/errorCodeMeanings.ts, the generated reference, and all 14 translations.The
tileBodymove from the previous round is reverted.tiles.tsstays within 200 lines because its body record is now written with grouped spreads, the fixture style.Proof
The policy is tested once, in
cluster/checked.test.ts:null; an optional 401 is refused.Each call site's own file tests only what that site adds:
gpu/page/httpSource.test.ts: the cache asks a 404 once, and retries a 503 while reportingstatus: 503.streaming/pageRefusal.test.ts: a 404 or 403 page gets one attempt, a sticky failureafter one attempt.host/prepared/imagesRead.test.ts: a 404 image is null and logged by address; a 503 then 200 image is decoded after two requests.lighting/importedLightsRead.test.ts: a 404 or 403 gives no lights; a 503 (asked twice) or a 401 (asked once) fails the read by address; an abort during the body read rejects.gpu/page/pages.test.ts: a hostPageSourcefailing with a plainError(no status) is still asked twice.physics/tileFetch.test.ts:physics.json404 or 403 collides nowhere.softBodiesstill opens.physics.json, aborts with no failure.Fixtures:
cluster/answers.fixture.ts(answering, withuntilAbortedalso used bycluster/pages.test.ts).modelFiles, shared bystubFetch.serveFiles(host/prepared/decodedImages.fixture.ts).Gates, all green:
pnpm run check:changedpnpm run test:changed(1620 pass, 0 fail)pnpm run validate --group quick, which includescheck:i18npnpm run validate --group typescriptnode scripts/check-pr-size.ts(592 hand-written lines added, limit 600)The branch merges cleanly with
origin/develop. Nothing was run in a browser.Local review before push
ifs instead ofunlessLeft;ONE_REQUESTfor tiles, which stops the doubled request per update; the soft-settings read takes the opening's signal.checked: every refused body let go;transientinlined;content-typeread once;credentialsremoved (default only).refusedStatus.checked.test.tsonrefusedWith/answering; per-site tests reduced to what each site adds; one abort helper; one physics fetch stub; the decode stub split out;URL_renamedLIGHTS.tileBodymove reverted.checkedinto a module with no dependencies so the wasm fetches could follow. That is outside Every engine fetch goes through the one HTTP loader: retry, abort and named errors #765.retriablerule shared bycheckedand the page cache; the code meaning reworded in every translation.softs.openlost its default for a file withoutsoftBodies, which would throw a rawTypeError. Now?? [], with a test.retriableErrorfor the three retry loops.PAGE_HTTP_<n>messages.stubFetchrestorefetchthrought.mock. It is test-only and would touch every physics test.maxAttemptsannounced before a 4xx stops the loop; a 503 then a network failure reportsstatus: null;refusedStatusrelies oninstanceof EngineError.indexOfintiles.ts:110, not reachable; the spreadwriter.addliteral kept to fit 200 lines.checked's two overloads andFetchPolicyreplaced by anattemptsnumber and oneoptionalFilewrapper (a 4xx is one request, so the count is unchanged);cookedBytestakesattempts; the test predicaterefusedWithreads the status throughrefusedStatus.triedin the page streamer (not derivable once the loop runs out); the spreadwriter.addliteral (200-line limit);stubFetchrestore and acheckedByteshelper (low cost, outside the batch); a retry wrapper for the three loops (they also retry non-HTTP failures); the<img>fallback for a platform withoutcreateImageBitmap(unchanged code); aretirehelper for two splice lines.refusedStatusreads a status only from aRESOURCE_HTTP_ERROR(anINVALID_JSON_RESPONSEcarrying the 200 it answered no longer counts as a 4xx);world.physics.errorlistsRESOURCE_HTTP_ERROR, in the source and all 14 translations;httpPageSourcekeeps the one-line summary its translations give (ENGINE.md holds the detail); tests for a 5xx image, a refusedlights.json(503, 401) and a plainPageSourcefailure, the first two failing ondevelop.develop(the documented "next update" rule).b287d6893..HEAD, 8 findings.retriablerule;checkedwaits theRetry-After(seconds or an HTTP date) before its next request, abortable.checked.test.tstests a 408 then a 200, a 429 then a 200, and thatRetry-Afteris waited on, both forms, undert.mock.timerswith no real delay.docs/SDK.mdupdated; the error-code meaning ("asked again first when the failure may pass") still holds, so no translation changed. The call-site tests were trimmed to stay under the size gate.cookedBytesleaves the default request count tocheckedagain; SDK.md no longer claims that the readers with their own retry (page streamer, GPU page cache, physics tiles) wait theRetry-After.Retry-After: the header is not carried in the error. Left to the CTO.httpSource.test.ts.Retry-Afterround: the wait lets go of its listener, its tests use mock timers and fail without it, docs/SDK.md is true. Fixed: a read already aborted before its wait now rejects at once, and a test aborts a 3600 s wait (it fails in 1 s when the wait ignores the abort). Left to the CTO:Retry-Afteris uncapped, so a load with no signal waits as long as the server asks; no engine constant bounds it, and a new cap would take the diff over 600 lines.credentialsleft out by the path note; decisions A (403 absent for optional reads) and B (streaming stops at a 4xx) each tested; tests for every call site; codes documented in SDK.md, ENGINE.md, the API reference and all 14 translations; no caller matchesPAGE_HTTP_; thePHYSICS_FAILEDcodes left are for other failures; the two wasm fetches rightly left out;Closes #765.Not proven / left out
Retry-Afteris honoured only bychecked's own second request. The page streamer, the GPU page cache and the physics tiles retry a 408/429 without waiting it. The wait has no cap, and the date form is measured against the client's clock.Lead verification
checkedfor the five call sites; the physics tiles take the model's abort signal): delivered inpackages/sdk-browser/src/cluster/pages.ts:7-60(checked(url, signal, attempts),optionalFile, oneretriablerule: a network error, a 5xx, a 408 or a 429, theRetry-Afterwaited, abortable), used bygpu/page/pages.ts:184,host/prepared/images.ts:30,lighting/importedLights.ts:35,physics/tilePlace.ts:50,58, and bystreaming/fetch.ts:35(CTO decision B); proved bya cache resource the server refuses once (503) is asked again and the cache opens,a resource still failing after its second request is refused by its address,an abort ends a read, even in its Retry-After wait,a timeout (408) or a rate limit (429) is asked again and answers,a refusal asking to wait (Retry-After, seconds or a date) is asked again once waited, and per sitea page the server does not hold (404) is refused by its address, the cache asking it once,a page read a busy server refuses once (503) is asked again by the cache, its status reported,an image a busy server refuses once (503) is asked again and decoded,a lights read aborted while its body arrives rejects, never answering no lights,a tile a busy server refuses (503) is asked once per update: the next one brings it in,a model leaving while its physics.json or a tile is on its way lets the read go, no failure,a page the server refuses (404, 403) is asked once, never the three attempts of a 5xx.optional: delivered asoptionalFile(a 404 or, per CTO decision A, a 403 answers null; a required read fails on either); proved byan optional file the server lacks (404) or hides (403) answers null, asked once,an optional file refused otherwise (401) is refused by its address, asked once,a cache without its lights — a 404, or the 403 of a store that hides it — has noneanda model compiled before the cook — a 404, or the 403 of a store that hides it — collides nowhere.credentials: not delivered, by the path note on the issue (its only use was fetch's own default, so no user).PAGE_HTTP_<n>,PHYSICS_FAILEDfor a file →RESOURCE_HTTP_ERROR): delivered indocs/SDK.md,docs/ENGINE.md, the API reference and its 14 translations (world.physics.errorincluded); proved bycheck:i18nand the reference tests; no caller left on the old codes.Retry-After) recorded on the issue.retriablerule where there were six fetch policies; the engine's own wasm fetches stay out (not a model's file), stated in the body.docs/SDK.md,docs/ENGINE.md, API reference and translations.in reviewset;to measureafter the merge (CTO rule P15: no branch proof for this 🟠, no image or budget change). Two reviewers said OK (the CTO's second-reviewer rule). Streaming without holes: rules and objectives for geometry, memory and shadows #483 checklist and CONTRIBUTING.md §Streaming, memory and shadows: no new per-frame work; a 4xx tile is no longer asked every frame; one declared limit:Retry-Afteris not capped (for the CTO).