From 76c064bd31b4daffac8a87c4614a21585bcc5ea5 Mon Sep 17 00:00:00 2001 From: Florian Wilde Date: Wed, 9 Sep 2026 22:46:09 +0200 Subject: [PATCH] fix(git): haengengebliebene Teil-Abrufe einmal nachschlagen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Behebt florian.wilde/nebula#23. Beobachtet an der laufenden Instanz: zwei von acht Teil-Abrufen liefen in eine Zeitueberschreitung nach 15 s, waehrend dieselben Aufrufe von aussen einzeln in 80 ms antworteten. Nicht die Endpunkte sind langsam - die Achterrunde erwuergt sich gelegentlich selbst. Daraus folgt die Maßnahme, und zwar genau diese: was haengen bleibt, bekommt einen zweiten Versuch, und der laeuft NICHT wieder im Schwung mit. Die Nachschlaege haengen an einer Kette und gehen nacheinander hinaus. Die Aufrufstellen in den drei Clients bleiben unberuehrt - hol() liefert weiterhin den endgueltigen Wert. Das war der Grund, es in Teilabrufe zu loesen statt hinter dem Promise.all: dort haette jeder Client acht Variablen neu zuweisen muessen. ZWEI GRENZEN, BEIDE ABSICHTLICH Nachgeschlagen wird nur bei 408 und bei 0 - Zeitueberschreitung und eine Verbindung, die gar nicht zustande kam. Ein 403 wird durch Wiederholen nicht besser, ein 429 durch sofortiges Wiederholen schlimmer, ein 404 bleibt ein 404. Und hoechstens zweimal je Abgleich. Acht Zeitueberschreitungen zu je 15 Sekunden waeren zwei Minuten, in denen "Jetzt synchronisieren" wie ein Absturz aussieht. Wer weg ist, soll schnell als weg erkannt werden. Klappt auch der zweite Versuch nicht, steht das am Repository - mit dem Vermerk "(auch im zweiten Versuch)", damit erkennbar bleibt, dass es kein Zufall war. NICHT GEMACHT: die acht Aufrufe entzerren. Das waere eine Vermutung ueber die Ursache. Bleibt das Problem, sagt der Vermerk genau das, und dann ist die Entzerrung dran. AM RANDE BEHOBEN Den beiden afterEach-Bloecken in http.test.ts fehlte restoreAllMocks. Die Spies zaehlten ueber Tests hinweg weiter; aufgefallen, weil die neuen Pruefungen an Aufrufzahlen haengen und statt 2 gleich 10 sahen. Gegengeprobt: mit abgeschaltetem Nachschlag fallen vier der sechs neuen Tests - die beiden, die das Ausbleiben pruefen, bleiben gruen. Refs #23 --- FEATURES.md | 9 +++ src/lib/git/http.test.ts | 138 +++++++++++++++++++++++++++++++++++++++ src/lib/git/http.ts | 58 ++++++++++++++++ 3 files changed, 205 insertions(+) diff --git a/FEATURES.md b/FEATURES.md index 0b2ce48..2013bd9 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -559,6 +559,15 @@ Diese Listen werden gleichzeitig geholt, und einzelne davon können scheitern, w der Rest durchgeht — ein 403 wegen fehlender Scopes, ein Rate-Limit, eine Zeitüberschreitung. +**Ein zweiter Versuch, bevor etwas als fehlend gilt.** Zeitüberschreitungen und +Verbindungen, die gar nicht zustande kamen, werden einmal nachgeschlagen — einzeln +statt im Schwung, weil genau das der beobachtete Fall war: zwei von acht Aufrufen +liefen in eine Zeitüberschreitung nach 15 Sekunden, während dieselben Aufrufe einzeln +in 80 Millisekunden antworteten. Nachgeschlagen wird höchstens zweimal je Abgleich — +acht Zeitüberschreitungen hintereinander wären zwei Minuten. Bei `403`, `404` und `429` +gibt es keinen zweiten Versuch: fehlende Rechte werden durch Wiederholen nicht besser, +ein Rate-Limit wird schlimmer. + **Ein gescheiterter Abruf ist keine leere Liste.** Der Unterschied steht am Snapshot: zu jedem Feld, das nicht geholt werden konnte, wird der Grund samt HTTP-Status festgehalten. Daraus folgt dreierlei: diff --git a/src/lib/git/http.test.ts b/src/lib/git/http.test.ts index ed485dc..157a8cf 100644 --- a/src/lib/git/http.test.ts +++ b/src/lib/git/http.test.ts @@ -51,6 +51,9 @@ describe("Teilabrufe", () => { afterEach(() => { vi.unstubAllEnvs(); + // Ohne das zaehlen die Spies ueber Tests hinweg weiter – und genau an + // Aufrufzahlen haengen die Pruefungen hier. + vi.restoreAllMocks(); }); it("meldet nichts, solange alles ankommt", async () => { @@ -125,3 +128,138 @@ describe("Teilabrufe", () => { expect(teil.fehler.commits).toBeTruthy(); }); }); + +/** + * Der zweite Versuch. + * + * Beobachtet an einer laufenden Instanz: zwei von acht Teil-Abrufen liefen + * in eine Zeitueberschreitung nach 15 s, waehrend dieselben Aufrufe von + * aussen einzeln in 80 ms antworteten. Nicht die Endpunkte sind langsam – + * die Achterrunde erwuergt sich gelegentlich selbst. + */ +describe("Teilabrufe: Nachschlag", () => { + beforeEach(() => { + vi.stubEnv("GIT_ALLOWED_HOSTS", "git.example.com"); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + // Ohne das zaehlen die Spies ueber Tests hinweg weiter – und genau an + // Aufrufzahlen haengen die Pruefungen hier. + vi.restoreAllMocks(); + }); + + /** Antwortet die ersten `fehlschlaege` Male mit Abbruch, danach mit Daten. */ + function zaehlend(fehlschlaege: number, status = 408) { + let aufruf = 0; + return vi.spyOn(globalThis, "fetch").mockImplementation(async () => { + aufruf += 1; + if (aufruf <= fehlschlaege) { + if (status === 408) { + const abbruch = new Error("aborted"); + abbruch.name = "AbortError"; + throw abbruch; + } + return new Response("nope", { status }); + } + return new Response(JSON.stringify(["da"]), { status: 200 }); + }); + } + + it("holt eine Zeitueberschreitung im zweiten Versuch nach", async () => { + const fetchSpy = zaehlend(1); + const teil = new Teilabrufe(); + + const wert = await teil.hol("branches", "https://git.example.com/a", []); + + expect(wert).toEqual(["da"]); + expect(fetchSpy).toHaveBeenCalledTimes(2); + // Geglueckt heisst: kein Eintrag. Sonst stuende am Repository ein + // Fehler, den es nicht mehr gibt. + expect(teil.fehler).toEqual({}); + expect(teil.unvollstaendig).toBe(false); + }); + + it("vermerkt es, wenn auch der zweite Versuch scheitert", async () => { + const fetchSpy = zaehlend(99); + const teil = new Teilabrufe(); + + const wert = await teil.hol("commits", "https://git.example.com/a", []); + + expect(wert).toEqual([]); + expect(fetchSpy).toHaveBeenCalledTimes(2); + expect(teil.fehler.commits).toContain("408"); + expect(teil.fehler.commits).toContain("zweiten Versuch"); + }); + + it("schlaegt bei 403 gar nicht erst nach", async () => { + // Fehlende Scopes werden durch Wiederholen nicht besser. + const fetchSpy = zaehlend(99, 403); + const teil = new Teilabrufe(); + + await teil.hol("tags", "https://git.example.com/a", []); + + expect(fetchSpy).toHaveBeenCalledTimes(1); + expect(teil.fehler.tags).not.toContain("zweiten Versuch"); + }); + + it("schlaegt bei 429 nicht nach", async () => { + // Sofortiges Wiederholen macht ein Rate-Limit schlimmer, nicht besser. + const fetchSpy = zaehlend(99, 429); + const teil = new Teilabrufe(); + + await teil.hol("issues", "https://git.example.com/a", []); + + expect(fetchSpy).toHaveBeenCalledTimes(1); + }); + + it("gibt hoechstens zwei Nachschlaege je Abgleich", async () => { + // Acht Zeitueberschreitungen zu je 15 s waeren zwei Minuten. Wer weg + // ist, soll schnell als weg erkannt werden. + const fetchSpy = zaehlend(99); + const teil = new Teilabrufe(); + + await Promise.all([ + teil.hol("branches", "https://git.example.com/a", []), + teil.hol("tags", "https://git.example.com/b", []), + teil.hol("commits", "https://git.example.com/c", []), + teil.hol("releases", "https://git.example.com/d", []), + ]); + + // Vier erste Versuche, aber nur zwei Nachschlaege. + expect(fetchSpy).toHaveBeenCalledTimes(6); + expect(Object.keys(teil.fehler).sort()).toEqual([ + "branches", + "commits", + "releases", + "tags", + ]); + }); + + it("reisst die Kette nicht ab, wenn ein Nachschlag scheitert", async () => { + // Der erste Nachschlag geht schief, der zweite muss trotzdem laufen. + let aufruf = 0; + const fetchSpy = vi.spyOn(globalThis, "fetch").mockImplementation(async () => { + aufruf += 1; + // 1 und 2 sind die ersten Versuche, 3 der erste Nachschlag (scheitert), + // 4 der zweite Nachschlag (glueckt). + if (aufruf <= 3) { + const abbruch = new Error("aborted"); + abbruch.name = "AbortError"; + throw abbruch; + } + return new Response(JSON.stringify(["da"]), { status: 200 }); + }); + + const teil = new Teilabrufe(); + const [erst, zweit] = await Promise.all([ + teil.hol("branches", "https://git.example.com/a", []), + teil.hol("tags", "https://git.example.com/b", []), + ]); + + expect(fetchSpy).toHaveBeenCalledTimes(4); + expect(erst).toEqual([]); + expect(zweit).toEqual(["da"]); + expect(Object.keys(teil.fehler)).toEqual(["branches"]); + }); +}); diff --git a/src/lib/git/http.ts b/src/lib/git/http.ts index e5d4612..b3a01f1 100644 --- a/src/lib/git/http.ts +++ b/src/lib/git/http.ts @@ -234,6 +234,16 @@ export async function getJsonSafe( } } +/** + * Wie viele zweite Versuche ein Abgleich hoechstens unternimmt. + * + * Zwei, nicht acht: jede Zeitueberschreitung kostet volle 15 Sekunden, und + * acht davon hintereinander waeren zwei Minuten, in denen "Jetzt + * synchronisieren" wie ein Absturz aussieht. Wer wirklich weg ist, soll + * schnell als weg erkannt werden. + */ +const NACHSCHLAG_MAX = 2; + /** Warum ein einzelner Teil-Abruf nicht geklappt hat. */ export type Teilfehler = Record; @@ -252,6 +262,18 @@ export type Teilfehler = Record; export class Teilabrufe { readonly fehler: Teilfehler = {}; + /** + * Nachschlaege haengen aneinander, damit sie nacheinander laufen. + * + * Die Runde selbst ist gleichzeitig – acht Aufrufe auf einmal. Genau das + * scheint sie gelegentlich zu erwuergen: beobachtet wurden zwei von acht + * mit Zeitueberschreitung nach 15 s, waehrend dieselben Aufrufe einzeln in + * 80 ms antworteten. Der zweite Versuch soll deshalb nicht wieder in + * denselben Schwung geraten. + */ + private kette: Promise = Promise.resolve(); + private nachgeschlagen = 0; + async hol( feld: string, url: string, @@ -262,11 +284,47 @@ export class Teilabrufe { const result = await getJson(url, options); return (result ?? fallback) as T; } catch (error) { + if (this.lohntNachschlag(error)) { + const zweiter = await this.nachschlag(url, options); + if (zweiter.ok) return (zweiter.wert ?? fallback) as T; + this.fehler[feld] = `${beschreibe(zweiter.fehler)} (auch im zweiten Versuch)`; + return fallback; + } this.fehler[feld] = beschreibe(error); return fallback; } } + /** + * Ein zweiter Versuch hilft nur bei Fehlern, die nichts ueber die Sache + * aussagen: Zeitueberschreitung (408) und eine Verbindung, die gar nicht + * zustande kam (0). Ein 403 wird durch Wiederholen nicht besser, ein 429 + * durch sofortiges Wiederholen schlimmer, und ein 404 bleibt ein 404. + */ + private lohntNachschlag(error: unknown): boolean { + if (this.nachgeschlagen >= NACHSCHLAG_MAX) return false; + if (!(error instanceof GitApiError)) return false; + return error.status === 408 || error.status === 0; + } + + private async nachschlag( + url: string, + options: FetchOptions, + ): Promise<{ ok: boolean; wert?: T; fehler?: unknown }> { + this.nachgeschlagen += 1; + + const versuch = this.kette.then(async () => getJson(url, options)); + // Auch ein Fehlschlag darf die Kette nicht abreissen lassen, sonst + // scheitern die folgenden Nachschlaege gleich mit. + this.kette = versuch.catch(() => undefined); + + try { + return { ok: true, wert: await versuch }; + } catch (fehler) { + return { ok: false, fehler }; + } + } + /** true, sobald mindestens ein Teil-Abruf gescheitert ist. */ get unvollstaendig(): boolean { return Object.keys(this.fehler).length > 0;