mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-10-30 09:16:55 +00:00
fix(studio): report a rejected read as a network reason, not an open failure (#4240)
* fix(studio): report a rejected read as a network reason, not an open failure `stage: "open"` is documented as `openComposition` threw — an unparseable composition, or OOM. Every event in that bucket on 0.8.56 and 0.8.57 carries a fetch-rejection message instead: "Failed to fetch" (28 users / 11 users), "Load failed", "NetworkError when attempting to fetch resource.". The cause is one missing catch. `readProjectFileOptional` awaits `fetch` with no try/catch, so a rejected request escapes the function and is caught by the effect's outer `.catch` — the handler written for `openComposition`. The read path has a five-member reason union precisely so this class of failure is diagnosable, and the one case that never produces a response was the case outside it. That made the largest SDK-session failure class in Studio (2.9% of session users on 0.8.56, 2.3% on 0.8.57) unaddressable: you cannot fix a network failure while it is reported as a composition that failed to parse. Now `readProjectFileOptional` catches its own rejection and returns `reason: "network"`, so it lands in `stage: read` with everything else, and `stage: open` means only what its comment says. Telemetry only. Nothing branches on the stage and no user-facing behaviour changes. Fifth split of this shape, after `ineligible_operation` (v0.8.49), `stage: read` (v0.8.51) and `block_detail` (#4214). Co-Authored-By: Claude Code <noreply@anthropic.com> * test(docs): point the catalog tab-row test at the page gutter #4238 moved the inset to `catalog-detail.test.ts` asserted the exact strings that #4238 deleted: `.hf-ve-tabs-row { margin: 20px 0 0 16px; }` and `.hf-ve-body { padding: 2rem 0 0 16px; }`. That PR removed both insets deliberately — its own comment says "the page gutter is the page's job, and a second 16px here put everything below the stage on a different left edge from the title, install block and stage above it" — and restored the gutter on `#content-area` in `docs/custom.css` instead. The test was left asserting the old mechanism, so main has been red on it since, unnoticed because the `Test` workflow does not run for a docs-only change. The invariant did not change: this content is still inset from the page edge. Only the mechanism did, so the test now asserts both halves of it — the two rules carry no inset of their own, and the gutter standing in for them is still present, at both its values. Asserting only the first would keep passing if the gutter were deleted outright, which is the regression the test exists to catch; both mutations were checked and both fail. Co-Authored-By: Claude Code <noreply@anthropic.com> --------- Co-authored-by: Claude Code <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Code
parent
c09594fef8
commit
5a8ee663f7
@@ -267,6 +267,34 @@ describe("useSdkSession unavailable telemetry", () => {
|
||||
await act(async () => root.unmount());
|
||||
});
|
||||
|
||||
// A fetch that REJECTS produces no response at all. It used to escape this
|
||||
// function and be caught by the effect's outer `.catch`, which reported it as
|
||||
// `stage: "open"` — a label that means openComposition threw. Every `stage:
|
||||
// open` event on 0.8.56/0.8.57 carries a fetch-rejection message, so the
|
||||
// largest failure class was a network problem reported as a parser one and
|
||||
// was unaddressable in that bucket.
|
||||
it("reports a rejected request as a read failure, not an open failure", async () => {
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(async () => {
|
||||
throw new TypeError("Failed to fetch");
|
||||
}),
|
||||
);
|
||||
const root = createRoot(document.createElement("div"));
|
||||
await act(async () => root.render(<Probe projectId="project-a" />));
|
||||
await flushAsyncEffects();
|
||||
|
||||
expect(trackMock).toHaveBeenCalledWith("sdk_session_unavailable", {
|
||||
stage: "read",
|
||||
reason: "network",
|
||||
});
|
||||
expect(trackMock).not.toHaveBeenCalledWith("sdk_session_unavailable", {
|
||||
stage: "open",
|
||||
error: expect.anything(),
|
||||
});
|
||||
await act(async () => root.unmount());
|
||||
});
|
||||
|
||||
it("separates an unexpected response shape from a failed request", async () => {
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
|
||||
@@ -16,12 +16,23 @@ import { addExternalFileReloadListener } from "./externalFileReloadBus";
|
||||
*
|
||||
* Every reason lives in this union so the full surface is readable from one
|
||||
* place — `absent_or_empty` included, even though it is a 2xx.
|
||||
*
|
||||
* `network` is a fetch that REJECTED — the request never produced a response.
|
||||
* It was not in this union until 2026-09-21, so it escaped `readProjectFileOptional`
|
||||
* and was caught by the effect's outer `.catch`, landing in `stage: "open"`.
|
||||
* That label means `openComposition` threw (unparseable composition, OOM), and
|
||||
* it is what the largest failure class was reported as. Every `stage: open`
|
||||
* event measured on 0.8.56 and 0.8.57 carries a fetch-rejection message —
|
||||
* "Failed to fetch", "Load failed", "NetworkError when attempting to fetch
|
||||
* resource." — so the class was a network problem filed under a parser one, and
|
||||
* unaddressable in that bucket.
|
||||
*/
|
||||
type ProjectFileReadFailure =
|
||||
| { ok: false; reason: "unsafe_path" }
|
||||
| { ok: false; reason: "http_error"; status: number }
|
||||
| { ok: false; reason: "missing_content" }
|
||||
| { ok: false; reason: "absent_or_empty" };
|
||||
| { ok: false; reason: "absent_or_empty" }
|
||||
| { ok: false; reason: "network" };
|
||||
|
||||
type ProjectFileReadResult = { ok: true; content: string } | ProjectFileReadFailure;
|
||||
|
||||
@@ -68,9 +79,20 @@ async function readProjectFileOptional(
|
||||
// and closes the CodeQL client-side-request-forgery flag). encodeURIComponent
|
||||
// already confines both values to single segments of this same-origin URL.
|
||||
if (path.includes("\0") || path.includes("..")) return { ok: false, reason: "unsafe_path" };
|
||||
const res = await fetch(
|
||||
`/api/projects/${encodeURIComponent(projectId)}/files/${encodeURIComponent(path)}?optional=1`,
|
||||
);
|
||||
let res: Response;
|
||||
try {
|
||||
res = await fetch(
|
||||
`/api/projects/${encodeURIComponent(projectId)}/files/${encodeURIComponent(path)}?optional=1`,
|
||||
);
|
||||
} catch {
|
||||
// The request never produced a response: offline, the dev server gone, a
|
||||
// CSP/Private-Network-Access block, or an extension rewriting fetch. Caught
|
||||
// here so it is reported as a READ failure — left to propagate, the effect's
|
||||
// outer catch reported it as `stage: "open"`, which claims the composition
|
||||
// failed to parse. Every `stage: open` event before this change was this
|
||||
// branch, so the label made the largest class unaddressable.
|
||||
return { ok: false, reason: "network" };
|
||||
}
|
||||
if (!res.ok) return { ok: false, reason: "http_error", status: res.status };
|
||||
const data = (await res.json()) as { content?: string };
|
||||
// `optional=1` answers a missing file with 200 + `content: ""`, so a
|
||||
@@ -299,6 +321,11 @@ export function useSdkSession(
|
||||
// openComposition threw (unparseable composition, OOM) — same total
|
||||
// bypass as the read failure above, but this one is a real defect
|
||||
// rather than a missing file. Carry the message; it is the only clue.
|
||||
//
|
||||
// A rejected read no longer reaches here: `readProjectFileOptional`
|
||||
// catches its own fetch rejection and answers `reason: "network"`, so
|
||||
// this stage now means what it says. Before that, every event in this
|
||||
// bucket was a network error wearing a parser's label.
|
||||
trackStudioEvent("sdk_session_unavailable", {
|
||||
stage: "open",
|
||||
error: error instanceof Error ? error.message : String(error),
|
||||
|
||||
@@ -567,12 +567,38 @@ test("a copy button holds its label and 'Copied' in one grid cell, so the swap n
|
||||
);
|
||||
});
|
||||
|
||||
test("the tab strip and tab content share the 16px inset the boxed panels (caption, tune) already use", async () => {
|
||||
// #4238 moved the left inset these two rules used to carry out to the page gutter,
|
||||
// so the tab strip, the panes under it and the stage above them all share one left
|
||||
// edge instead of the content below the stage sitting 16px further in. The
|
||||
// invariant is unchanged — this content is inset from the page edge — but the
|
||||
// mechanism moved, so assert both halves of it: the rules carry no inset of their
|
||||
// own, and the gutter standing in for them is still there. Asserting only the first
|
||||
// would keep passing if the gutter were deleted outright, which is the failure this
|
||||
// test exists to catch.
|
||||
test("the tab strip and tab content align to the page gutter, with no inset of their own", async () => {
|
||||
await renderDetail(["install"]);
|
||||
const source = readFileSync(
|
||||
join(dirname(fileURLToPath(import.meta.url)), "..", "docs", "snippets", "catalog-detail.jsx"),
|
||||
"utf8",
|
||||
const root = join(dirname(fileURLToPath(import.meta.url)), "..");
|
||||
const source = readFileSync(join(root, "docs", "snippets", "catalog-detail.jsx"), "utf8");
|
||||
assert.match(
|
||||
source,
|
||||
/\.hf-ve-tabs-row \{ margin: 20px 0 0; \}/,
|
||||
"tab strip carries no left inset",
|
||||
);
|
||||
assert.match(
|
||||
source,
|
||||
/\.hf-ve-body \{ padding: 2rem 0 0; \}/,
|
||||
"tab content carries no left inset",
|
||||
);
|
||||
|
||||
const css = readFileSync(join(root, "docs", "custom.css"), "utf8");
|
||||
assert.match(
|
||||
css,
|
||||
/header\.is-frame ~ #body-content #content-area \{\s*padding-left: 1\.25rem;/,
|
||||
"frame mode restores the page gutter the removed insets stood in for",
|
||||
);
|
||||
assert.match(
|
||||
css,
|
||||
/@media \(min-width: 1024px\) \{\s*header\.is-frame ~ #body-content #content-area \{\s*padding-left: 5\.5rem;/,
|
||||
"the gutter keeps its wide-viewport value",
|
||||
);
|
||||
assert.match(source, /\.hf-ve-tabs-row \{ margin: 20px 0 0 16px; \}/, "tab strip is inset 16px");
|
||||
assert.match(source, /\.hf-ve-body \{ padding: 2rem 0 0 16px; \}/, "tab content is inset 16px");
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user