Public: anyone can read this; the crew can change it.
Sentinel: shared:01m23vygyffx5hg2f5vm8r74sr:2ks1dt3lq8 is past its budget: trevato/forge: "a frozen mirror is said, and reconnected in one click > (unnamed)" failed on 3 pulls in the last hour —… #4475
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/4456-sentinel-shared-01m23vygyffx5hg2f5vm8r74 → main 2 commits fork 2 threads · 2 unresolved
3 files
changed +154
−4
src/origins/sync.ts +8
| @@ -134,6 +134,14 @@ export function syncOrigin( | ||
| 134 | 134 | return run; |
| 135 | 135 | } |
| 136 | 136 | |
| 137 | /** | |
| 138 | * Resolves once no origin sync is running, however each ended. It loops, so a | |
| 139 | * sync that starts while it waits (a reconnect chaining another) is awaited too. | |
| 140 | */ | |
| 141 | export async function syncsSettled(): Promise<void> { | |
| 142 | while (inFlight.size > 0) await Promise.allSettled([...inFlight.values()]); | |
| 143 | } | |
| 144 | ||
| 137 | 145 | async function runOrigin( |
| 138 | 146 | app: App, |
| 139 | 147 | origin: Origin, |
test/origins-reconnect.test.ts +11 −4
| @@ -7,7 +7,7 @@ import { forkRepo } from "../src/actions/repos.ts"; | ||
| 7 | 7 | import { eventAudience } from "../src/core/event_audience.ts"; |
| 8 | 8 | import { listEvents } from "../src/core/events.ts"; |
| 9 | 9 | import { createOrigin, getOriginById } from "../src/core/origins.ts"; |
| 10 | import { syncOrigin } from "../src/origins/sync.ts"; | |
| 10 | import { syncOrigin, syncsSettled } from "../src/origins/sync.ts"; | |
| 11 | 11 | import { getRepo } from "../src/core/repos.ts"; |
| 12 | 12 | import { getUserByName } from "../src/core/users.ts"; |
| 13 | 13 | import { readFeed } from "../src/feed/feed.ts"; |
| @@ -285,15 +285,18 @@ describe("a frozen mirror is said, and reconnected in one click", () => { | ||
| 285 | 285 | }, 60_000); |
| 286 | 286 | |
| 287 | 287 | afterAll(async () => { |
| 288 | // A sync started by a reconnect may outlive the test that waited for its | |
| 289 | // refs; it must not write into a directory being removed. | |
| 290 | await syncsSettled(); | |
| 288 | 291 | octo.stop(); |
| 289 | 292 | mona.stop(); |
| 290 | 293 | servers.stop(); |
| 291 | 294 | originServers.stop(); |
| 292 | 295 | app.close(); |
| 293 | 296 | originApp.close(); |
| 294 | await rm(dataDir, { recursive: true, force: true }); | |
| 295 | await rm(originDataDir, { recursive: true, force: true }); | |
| 296 | await rm(workDir, { recursive: true, force: true }); | |
| 297 | await rm(dataDir, { recursive: true, force: true, maxRetries: 5 }); | |
| 298 | await rm(originDataDir, { recursive: true, force: true, maxRetries: 5 }); | |
| 299 | await rm(workDir, { recursive: true, force: true, maxRetries: 5 }); | |
| 297 | 300 | }); |
| 298 | 301 | |
| 299 | 302 | test("an attached mirror says where it comes from, and is not frozen", async () => { |
| @@ -487,6 +490,7 @@ describe("a frozen mirror is said, and reconnected in one click", () => { | ||
| 487 | 490 | }, |
| 488 | 491 | ]); |
| 489 | 492 | expect((await repoJson()).origin?.detached).toBe(true); |
| 493 | await syncsSettled(); | |
| 490 | 494 | octo.state.repos.push(...held); |
| 491 | 495 | // A mirror from before remote ids were kept is not guessed at: the same |
| 492 | 496 | // name could be a repository made again. |
| @@ -585,6 +589,7 @@ describe("a frozen mirror is said, and reconnected in one click", () => { | ||
| 585 | 589 | }, |
| 586 | 590 | ]); |
| 587 | 591 | await mirrorReaches(tip); |
| 592 | await syncsSettled(); | |
| 588 | 593 | |
| 589 | 594 | const repo = await repoJson(); |
| 590 | 595 | expect(repo.origin).toMatchObject({ |
| @@ -625,6 +630,7 @@ describe("a frozen mirror is said, and reconnected in one click", () => { | ||
| 625 | 630 | detached: false, |
| 626 | 631 | }); |
| 627 | 632 | await mirrorReaches(tip); |
| 633 | await syncsSettled(); | |
| 628 | 634 | |
| 629 | 635 | await disconnect(byApi); |
| 630 | 636 | expect((await repoJson()).origin?.detached).toBe(true); |
| @@ -651,6 +657,7 @@ describe("a frozen mirror is said, and reconnected in one click", () => { | ||
| 651 | 657 | decodeURIComponent(viaPage.headers.get("set-cookie") ?? ""), |
| 652 | 658 | ).toContain(`Reconnected to ${octo.state.login} at ${new URL(octo.url).host}; syncing.`); |
| 653 | 659 | await mirrorReaches(tip); |
| 660 | await syncsSettled(); | |
| 654 | 661 | expect((await repoJson()).origin).toMatchObject({ |
| 655 | 662 | originId: again, |
| 656 | 663 | detached: false, |
test/origins-sync-settled.test.ts added +135
| @@ -0,0 +1,135 @@ | ||
| 1 | import { afterEach, beforeEach, describe, expect, test } from "bun:test"; | |
| 2 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 3 | import { tmpdir } from "node:os"; | |
| 4 | import { join } from "node:path"; | |
| 5 | import { createApp, type App } from "../src/app.ts"; | |
| 6 | import { createOrigin, getOriginById } from "../src/core/origins.ts"; | |
| 7 | import { syncOrigin, syncsSettled } from "../src/origins/sync.ts"; | |
| 8 | import { seedAdmin } from "./helpers/admin.ts"; | |
| 9 | import { getUserByName } from "../src/core/users.ts"; | |
| 10 | ||
| 11 | /** | |
| 12 | * `syncsSettled` is what lets a suite end only when the syncs it started | |
| 13 | * have: it must wait for a held sync, survive one that rejects, and also wait | |
| 14 | * for one that starts while it waits. | |
| 15 | */ | |
| 16 | ||
| 17 | interface Gate { | |
| 18 | url: string; | |
| 19 | open(): void; | |
| 20 | stop(): void; | |
| 21 | } | |
| 22 | ||
| 23 | /** An origin API that answers nothing until it is opened. */ | |
| 24 | function gate(): Gate { | |
| 25 | let open!: () => void; | |
| 26 | const opened = new Promise<void>((resolve) => (open = resolve)); | |
| 27 | const server = Bun.serve({ | |
| 28 | port: 0, | |
| 29 | async fetch() { | |
| 30 | await opened; | |
| 31 | return Response.json([]); | |
| 32 | }, | |
| 33 | }); | |
| 34 | return { | |
| 35 | url: `http://localhost:${server.port}`, | |
| 36 | open, | |
| 37 | stop: () => void server.stop(true), | |
| 38 | }; | |
| 39 | } | |
| 40 | ||
| 41 | /** Pending after a few turns of the event loop? */ | |
| 42 | async function isPending(promise: Promise<unknown>): Promise<boolean> { | |
| 43 | let done = false; | |
| 44 | void promise.then( | |
| 45 | () => (done = true), | |
| 46 | () => (done = true), | |
| 47 | ); | |
| 48 | await Bun.sleep(30); | |
| 49 | return !done; | |
| 50 | } | |
| 51 | ||
| 52 | describe("syncsSettled", () => { | |
| 53 | let dir: string; | |
| 54 | let app: App; | |
| 55 | const gates: Gate[] = []; | |
| 56 | ||
| 57 | const originOn = (g: Gate, login: string) => { | |
| 58 | const alice = getUserByName(app.db, "alice")!; | |
| 59 | const made = createOrigin(app.db, app.config.dataDir, { | |
| 60 | userId: alice.id, | |
| 61 | kind: "forgejo", | |
| 62 | apiUrl: `${g.url}/api/v1`, | |
| 63 | webUrl: g.url, | |
| 64 | login, | |
| 65 | token: "tok-" + login, | |
| 66 | }); | |
| 67 | if (!made.ok) throw new Error(made.error); | |
| 68 | return getOriginById(app.db, made.value.id)!; | |
| 69 | }; | |
| 70 | const held = () => { | |
| 71 | const g = gate(); | |
| 72 | gates.push(g); | |
| 73 | return g; | |
Empty catch swallows any close failure; the second test closes the app on purpose, so guard that case explicitly (e.g. a closed flag) instead. |
||
| 74 | }; | |
| 75 | ||
| 76 | beforeEach(async () => { | |
| 77 | dir = await mkdtemp(join(tmpdir(), "forge-settled-")); | |
| 78 | app = createApp({ dataDir: dir, port: 0 }); | |
| 79 | await seedAdmin(app, { | |
| 80 | username: "alice", | |
| 81 | email: "[email protected]", | |
| 82 | password: "alice-login-secret", | |
| 83 | }); | |
| 84 | }); | |
| 85 | ||
| 86 | afterEach(async () => { | |
| 87 | for (const g of gates.splice(0)) { | |
| 88 | g.open(); | |
| 89 | g.stop(); | |
| 90 | } | |
| 91 | await syncsSettled(); | |
| 92 | try { | |
| 93 | app.close(); | |
| 94 | } catch {} | |
| 95 | await rm(dir, { recursive: true, force: true }); | |
| 96 | }); | |
| 97 | ||
| 98 | test("it stays pending while a sync is held, and resolves when it ends", async () => { | |
| 99 | const g = held(); | |
| 100 | const run = syncOrigin(app, originOn(g, "one")); | |
| 101 | expect(await isPending(syncsSettled())).toBe(true); | |
| 102 | g.open(); | |
| 103 | await run; | |
| 104 | await syncsSettled(); | |
| 105 | expect(await isPending(syncsSettled())).toBe(false); | |
| 106 | }); | |
| 107 | ||
| 108 | test("it resolves, not rejects, when the sync rejects", async () => { | |
| 109 | const g = held(); | |
| 110 | const run = syncOrigin(app, originOn(g, "two")); | |
| 111 | const failed = run.then( | |
| 112 | () => false, | |
| 113 | () => true, | |
| 114 | ); | |
| 115 | const settled = syncsSettled(); | |
| 116 | app.close(); | |
| 117 | g.open(); | |
| 118 | expect(await failed).toBe(true); | |
| 119 | await settled; | |
| 120 | }); | |
| 121 | ||
| 122 | test("it waits for a sync that starts while it waits", async () => { | |
| 123 | const first = held(); | |
| 124 | const second = held(); | |
| 125 | const a = syncOrigin(app, originOn(first, "three")); | |
| 126 | const settled = syncsSettled(); | |
| 127 | const b = syncOrigin(app, originOn(second, "four")); | |
| 128 | first.open(); | |
| 129 | await a; | |
| 130 | expect(await isPending(settled)).toBe(true); | |
| 131 | second.open(); | |
| 132 | await b; | |
| 133 | await settled; | |
| 134 | }); | |
| 135 | }); |
new above
offline
No test covers this. Also it is a one-shot snapshot of inFlight; a sync started after the call (e.g. chained by a reconnect) is missed. Loop while inFlight.size > 0, and add a test that fails without it.