Public: anyone can read this; the crew can change it.
Errors on reports, the sandbox page and the fleet graph are §5's field error, not .error-note #5089
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/5070-errors-on-reports-the-sandbox-page-and-t → main 1 commit fork 1 thread · 1 unresolved
6 files
changed +68
−17
src/web/pages/fleet-graph.ts +2 −5
| @@ -75,6 +75,7 @@ export function fleetGraphPage(ctx: Ctx): Response { | ||
| 75 | 75 | label: "Repositories", |
| 76 | 76 | name: "repo", |
| 77 | 77 | id: "fleet-repos", |
| 78 | ...(asked.ok ? {} : { error: asked.error }), | |
| 78 | 79 | value: names, |
| 79 | 80 | placeholder: |
| 80 | 81 | "owner/name owner/name — empty is everything you can write that spent", |
| @@ -84,11 +85,7 @@ export function fleetGraphPage(ctx: Ctx): Response { | ||
| 84 | 85 | })} |
| 85 | 86 | ${btn("Draw", { type: "submit", variant: "secondary" })} |
| 86 | 87 | </form> |
| 87 | ${graph === undefined | |
| 88 | ? html`<p class="error-note" role="alert"> | |
| 89 | ${asked.ok ? "" : asked.error} | |
| 90 | </p>` | |
| 91 | : fleetGraphSection(graph)}`, | |
| 88 | ${graph === undefined ? null : fleetGraphSection(graph)}`, | |
| 92 | 89 | scriptJs: fleetGraphClientJs, |
| 93 | 90 | }, |
| 94 | 91 | graph === undefined ? 422 : 200, |
src/web/pages/reports.ts +20 −8
| @@ -60,7 +60,7 @@ function reportForm( | ||
| 60 | 60 | ctx: Ctx, |
| 61 | 61 | repo: Repo, |
| 62 | 62 | values: FormValues, |
| 63 | error?: string, | |
| 63 | error?: { on?: "title" | "body"; message: string }, | |
| 64 | 64 | status = 200, |
| 65 | 65 | ): Response { |
| 66 | 66 | const base = repoBase(repo); |
| @@ -68,6 +68,9 @@ function reportForm( | ||
| 68 | 68 | { |
| 69 | 69 | ctx, |
| 70 | 70 | title: `Report a problem · ${repo.ownerName}/${repo.name}`, |
| 71 | ...(error && !error.on | |
| 72 | ? { flash: { tone: "danger" as const, message: error.message } } | |
| 73 | : {}), | |
| 71 | 74 | crumbs: crumbsFor(repo), |
| 72 | 75 | body: html`${repoHeader(ctx, repo, "issues", { |
| 73 | 76 | compact: true, |
| @@ -81,7 +84,6 @@ function reportForm( | ||
| 81 | 84 | nothing acts on it before they do. |
| 82 | 85 | </p> |
| 83 | 86 | <form class="mt" method="post" action="${base}/report"> |
| 84 | ${when(error, (m) => html`<p class="error-note">${m}</p>`)} | |
| 85 | 87 | ${formField({ |
| 86 | 88 | label: "What is wrong", |
| 87 | 89 | name: "title", |
| @@ -89,6 +91,7 @@ function reportForm( | ||
| 89 | 91 | required: true, |
| 90 | 92 | autofocus: true, |
| 91 | 93 | hint: `One line, at most ${TITLE_MAX} characters.`, |
| 94 | ...(error?.on === "title" ? { error: error.message } : {}), | |
| 92 | 95 | })} |
| 93 | 96 | ${formField({ |
| 94 | 97 | label: "What happened", |
| @@ -96,6 +99,7 @@ function reportForm( | ||
| 96 | 99 | type: "textarea", |
| 97 | 100 | rows: 8, |
| 98 | 101 | value: values.body, |
| 102 | ...(error?.on === "body" ? { error: error.message } : {}), | |
| 99 | 103 | hint: `Plain text, at most ${BODY_MAX} characters. What you did, what you saw, what you expected.`, |
| 100 | 104 | })} |
| 101 | 105 | <input type="hidden" name="page" value="${values.page}" /> |
| @@ -152,11 +156,17 @@ function reportBody(ctx: Ctx, repo: Repo): Response { | ||
| 152 | 156 | }); |
| 153 | 157 | } |
| 154 | 158 | |
| 155 | const REFUSED: Record<string, string> = { | |
| 156 | empty_title: "Say in one line what is wrong.", | |
| 157 | title_too_long: `That line is too long (${TITLE_MAX} characters at most).`, | |
| 158 | body_too_long: `That is too long (${BODY_MAX} characters at most).`, | |
| 159 | queue_full: "Many reports are waiting here already. Try again later.", | |
| 159 | const REFUSED: Record<string, { on?: "title" | "body"; message: string }> = { | |
| 160 | empty_title: { on: "title", message: "Say in one line what is wrong." }, | |
| 161 | title_too_long: { | |
| 162 | on: "title", | |
| 163 | message: `That line is too long (${TITLE_MAX} characters at most).`, | |
| 164 | }, | |
| 165 | body_too_long: { | |
| 166 | on: "body", | |
| 167 | message: `That is too long (${BODY_MAX} characters at most).`, | |
| 168 | }, | |
| 169 | queue_full: { message: "Many reports are waiting here already. Try again later." }, | |
| 160 | 170 | }; |
| 161 | 171 | |
| 162 | 172 | // POST /:owner/:name/report |
| @@ -185,7 +195,9 @@ export async function reportSubmit(ctx: Ctx): Promise<Response> { | ||
| 185 | 195 | ctx, |
| 186 | 196 | repo, |
| 187 | 197 | values, |
| 188 | `Too many reports just now. Try again in ${refusal.retryAfter} seconds.`, | |
| 198 | { | |
| 199 | message: `Too many reports just now. Try again in ${refusal.retryAfter} seconds.`, | |
| 200 | }, | |
| 189 | 201 | 429, |
| 190 | 202 | ); |
| 191 | 203 | response.headers.set("retry-after", String(refusal.retryAfter)); |
src/web/studio/sandbox.ts +5 −4
| @@ -8,6 +8,7 @@ | ||
| 8 | 8 | import type { Run } from "../../core/runs.ts"; |
| 9 | 9 | import { placementWords } from "../../checks/placement.ts"; |
| 10 | 10 | import { html, type Html } from "../html.ts"; |
| 11 | import { badge } from "../ui.ts"; | |
| 11 | 12 | |
| 12 | 13 | /** Where a check ran, when that was a cluster job; nothing for the default. */ |
| 13 | 14 | export function placementLine(run: Run): Html | null { |
| @@ -24,11 +25,11 @@ export function sandboxLine(run: Run): Html | null { | ||
| 24 | 25 | ? "Sandboxed: the shell was held to the policy." |
| 25 | 26 | : "Contained: the repository's code ran in a cell of its own, with limits."} |
| 26 | 27 | </p>`; |
| 27 | return html`<p class="error-note" id="run-sandbox"> | |
| 28 | return html`<p class="small" id="run-sandbox"> | |
| 29 | ${badge(agent ? "Unsandboxed" : "Uncontained", { tone: "danger" })} | |
| 28 | 30 | ${agent |
| 29 | ? "Unsandboxed: the shell ran unrestricted" | |
| 30 | : "Uncontained: the repository's code ran unrestricted"}${run.sandboxNote === | |
| 31 | "" | |
| 31 | ? "The shell ran unrestricted" | |
| 32 | : "The repository's code ran unrestricted"}${run.sandboxNote === "" | |
| 32 | 33 | ? "" |
| 33 | 34 | : html` (${run.sandboxNote})`}. |
| 34 | 35 | </p>`; |
test/crew-sandbox.test.ts +2
| @@ -192,6 +192,8 @@ describe("the sandbox on a run's record", () => { | ||
| 192 | 192 | expect(text).toContain("Unsandboxed"); |
| 193 | 193 | expect(text).toContain("<b>bwrap</b> is not installed"); |
| 194 | 194 | expect(text).not.toContain("<b>"); |
| 195 | expect(text).not.toContain("error-note"); | |
| 196 | expect(text).toContain('data-variant="danger"'); | |
| 195 | 197 | |
| 196 | 198 | const on = agentRun(); |
| 197 | 199 | noteRunProgress(app.db, on.id, { sandbox: { enforced: true, note: "" } }); |
test/fleet-graph.test.ts +2
| @@ -349,6 +349,8 @@ describe("the graph", () => { | ||
| 349 | 349 | const body = await res.text(); |
| 350 | 350 | expect(body).not.toContain("<script>x</script>"); |
| 351 | 351 | expect(body).toContain("<script>"); |
| 352 | expect(body).not.toContain("error-note"); | |
| 353 | expect(body).toMatch(/id="fleet-repos"[^>]*aria-invalid="true"[\s\S]*?id="fleet-repos-err"/); | |
| 352 | 354 | }); |
| 353 | 355 | |
| 354 | 356 | test("the API answers the same numbers, and refuses a bad name", async () => { |
test/reports.test.ts +37
| @@ -547,6 +547,43 @@ describe("reports from outside", () => { | ||
| 547 | 547 | expect(res.status).toBe(429); |
| 548 | 548 | expect(((await res.json()) as any).error).toBe("queue_full"); |
| 549 | 549 | }); |
| 550 | ||
| 551 | const send = (fields: Record<string, string>) => | |
| 552 | fetch(`${base}/alice/game/report`, { | |
| 553 | method: "POST", | |
| 554 | headers: { "content-type": "application/x-www-form-urlencoded" }, | |
| 555 | body: new URLSearchParams(fields).toString(), | |
| 556 | redirect: "manual", | |
| 557 | }); | |
| 558 | ||
| 559 | test("a title or body refusal sits in its own field", async () => { | |
| 560 | app.db.run("DELETE FROM report"); | |
| 561 | for (const [fields, id] of [ | |
| 562 | [{ title: "", body: "b" }, "f-title"], | |
| 563 | [{ title: "t", body: "x".repeat(8_001) }, "f-body"], | |
| 564 | ] as const) { | |
| 565 | const res = await send(fields); | |
| 566 | expect(res.status).toBe(422); | |
| 567 | const page = await res.text(); | |
| 568 | expect(page).not.toContain("error-note"); | |
| 569 | expect(page).toMatch(new RegExp(`id="${id}"[^>]*aria-invalid="true"[\\s\\S]*?class="error"`)); | |
| 570 | } | |
| 571 | }); | |
| 572 | ||
| 573 | test("a full queue is the page's flash", async () => { | |
| 574 | app.db.run("DELETE FROM report"); | |
| 575 | const insert = app.db.query( | |
| 576 | "INSERT INTO report (id, repo_id, title, body) VALUES (?1, ?2, 't', '')", | |
| 577 | ); | |
| 578 | for (let i = 0; i < 100; i++) insert.run(`fillform${i}`, game.id); | |
| 579 | const res = await send({ title: "one more", body: "" }); | |
| 580 | expect(res.status).toBe(429); | |
| 581 | const page = await res.text(); | |
| 582 | expect(page).not.toContain("error-note"); | |
| 583 | expect(page).toContain('data-tone="danger"'); | |
| 584 | expect(page).toContain("Many reports are waiting here already"); | |
| 585 | app.db.run("DELETE FROM report"); | |
| 586 | }); | |
| 550 | 587 | }); |
| 551 | 588 | |
| 552 | 589 | describe("the title of an issue made from a report", () => { |
new above
offline
Not blocking: this rate-limit re-render (429 + retry-after, now a flash) is the one refusal with no test of its own. It shares queue_full's path, so it's covered in effect.