diff --git a/src/bin/cli.ts b/src/bin/cli.ts index 9fb4321..b932f1a 100644 --- a/src/bin/cli.ts +++ b/src/bin/cli.ts @@ -31,10 +31,13 @@ SCHULCLOUD_SYNC_DIR. Config file: ${configPath()} `; async function main(argv: string[]): Promise { - const [command, ...rest] = argv; - const flags = parseFlags(rest); + // A leading flag means no command was given: `schulcloud --help` must not be + // read as a command called "--help". + const hasCommand = argv[0] !== undefined && !argv[0].startsWith('-'); + const command = hasCommand ? argv[0] : undefined; + const flags = parseFlags(hasCommand ? argv.slice(1) : argv); - if (!command || command === 'help' || flags.help) { + if (!command || command === 'help' || flags.help || flags.h) { process.stdout.write(USAGE); return 0; } diff --git a/src/core/client.ts b/src/core/client.ts index 538f821..21dd834 100644 --- a/src/core/client.ts +++ b/src/core/client.ts @@ -15,6 +15,20 @@ import type { TaskContent, } from './types.ts'; +/** Statuses worth retrying: transient by definition, and every call here is a GET. */ +const RETRYABLE = new Set([429, 500, 502, 503, 504]); +const MAX_RETRIES = 3; + +/** Exponential backoff with jitter, so parallel workers do not retry in lockstep. */ +function backoffMs(attempt: number): number { + const base = 400 * 2 ** (attempt - 1); + return Math.round(base + Math.random() * base * 0.5); +} + +function delay(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + /** An API response outside the 2xx range, carrying the status for callers to branch on. */ export class SchulcloudApiError extends Error { readonly status: number; @@ -83,16 +97,44 @@ export class SchulcloudClient { } private async request(url: URL, accept: string): Promise { - const response = await fetch(url, { - headers: { Authorization: `Bearer ${this.config.jwt}`, Accept: accept }, - signal: AbortSignal.timeout(this.config.requestTimeoutMs), - redirect: 'follow', - }); - if (!response.ok) { + let lastError: unknown; + + for (let attempt = 0; attempt <= MAX_RETRIES; attempt++) { + if (attempt > 0) await delay(backoffMs(attempt)); + + let response: Response; + try { + response = await fetch(url, { + headers: { Authorization: `Bearer ${this.config.jwt}`, Accept: accept }, + signal: AbortSignal.timeout(this.config.requestTimeoutMs), + redirect: 'follow', + }); + } catch (error) { + // Connection reset or timeout: worth one more try, since every call + // here is an idempotent GET. + lastError = error; + if (attempt === MAX_RETRIES) throw error; + continue; + } + + if (response.ok) return response; + const body = await response.text().catch(() => ''); - throw new SchulcloudApiError(response.status, url.pathname + url.search, body); + const error = new SchulcloudApiError(response.status, url.pathname + url.search, body); + + // A crawl issues hundreds of requests and the instance answers some of + // them with a 503 front-page when it decides we are going too fast. + // Observed live: 4 of 26 course pages failed that way in one crawl, and + // all of them succeeded on a retry. 429 and the other gateway errors + // are the same kind of "come back shortly". + if (!RETRYABLE.has(response.status) || attempt === MAX_RETRIES) throw error; + + const retryAfter = Number(response.headers.get('retry-after')); + if (Number.isFinite(retryAfter) && retryAfter > 0) await delay(retryAfter * 1000); + lastError = error; } - return response; + + throw lastError instanceof Error ? lastError : new Error('request failed'); } /** Authenticated GET returning JSON. `path` is absolute, e.g. `/api/v3/courses`. */ diff --git a/test/client.test.ts b/test/client.test.ts new file mode 100644 index 0000000..b96b6b2 --- /dev/null +++ b/test/client.test.ts @@ -0,0 +1,82 @@ +import assert from 'node:assert/strict'; +import { afterEach, describe, it } from 'node:test'; +import { SchulcloudClient, SchulcloudApiError } from '../src/core/client.ts'; + +/** + * Retry behaviour, driven by a stubbed fetch. Observed live: a full crawl made + * the instance answer 4 of 26 course pages with a 503 front-page, all of which + * succeeded on retry — so this is the difference between a complete index and a + * quietly incomplete one. + */ +const config = { + baseUrl: 'https://example.test', + jwt: 'x', + authToken: undefined, + port: 1, + bindHost: '127.0.0.1', + maxDownloadBytes: 1000, + maxExtractedChars: 1000, + requestTimeoutMs: 1000, + keepaliveIntervalMs: 0, + databaseUrl: undefined, + mirrorDir: '/tmp', + mirrorMaxBytes: 1000, + crawlIntervalMs: 0, +}; + +const realFetch = globalThis.fetch; +afterEach(() => { + globalThis.fetch = realFetch; +}); + +function stubFetch(responses: (Response | Error)[]): () => number { + let calls = 0; + globalThis.fetch = (async () => { + const next = responses[calls++] ?? responses.at(-1)!; + if (next instanceof Error) throw next; + return next; + }) as typeof fetch; + return () => calls; +} + +const json = (body: unknown, status = 200) => + new Response(JSON.stringify(body), { status, headers: { 'content-type': 'application/json' } }); + +describe('SchulcloudClient retries', () => { + it('retries a 503 and succeeds', async () => { + const calls = stubFetch([json({}, 503), json({ school: { id: 's' } })]); + const client = new SchulcloudClient(config); + await client.me(); + assert.equal(calls(), 2); + }); + + it('retries transient network errors', async () => { + const calls = stubFetch([new Error('ECONNRESET'), json({ school: { id: 's' } })]); + await new SchulcloudClient(config).me(); + assert.equal(calls(), 2); + }); + + it('does not retry a 401 — a dead token will not recover', async () => { + const calls = stubFetch([json({}, 401)]); + await assert.rejects(() => new SchulcloudClient(config).me(), (error: SchulcloudApiError) => { + assert.equal(error.status, 401); + return true; + }); + assert.equal(calls(), 1, 'retrying an expired token just wastes time'); + }); + + it('does not retry a 404', async () => { + const calls = stubFetch([json({}, 404)]); + await assert.rejects(() => new SchulcloudClient(config).me()); + assert.equal(calls(), 1); + }); + + it('gives up after the retry budget and reports the real status', async () => { + const calls = stubFetch([json({}, 503)]); + await assert.rejects(() => new SchulcloudClient(config).me(), (error: SchulcloudApiError) => { + assert.equal(error.status, 503); + return true; + }); + assert.equal(calls(), 4, 'one attempt plus three retries'); + }); +});