From f41e3d274c9c27468aec17514a27b43cbd8eabe0 Mon Sep 17 00:00:00 2001 From: DaxServer Date: Sun, 5 Jul 2026 12:23:50 +0200 Subject: [PATCH 1/2] fix: retry MediaWiki upload on 5xx instead of failing immediately Co-Authored-By: Claude Sonnet 5 --- backend/src/__tests__/upload.worker.test.ts | 10 +++++++++- backend/src/__tests__/uploadClient.test.ts | 16 ++++++++++++++++ backend/src/core/errors.ts | 7 +++++++ backend/src/mediawiki/client.ts | 7 ++++++- backend/src/workers/upload.worker.ts | 4 +++- 5 files changed, 41 insertions(+), 3 deletions(-) diff --git a/backend/src/__tests__/upload.worker.test.ts b/backend/src/__tests__/upload.worker.test.ts index 402a607..e80745d 100644 --- a/backend/src/__tests__/upload.worker.test.ts +++ b/backend/src/__tests__/upload.worker.test.ts @@ -1,4 +1,9 @@ -import { HashLockError, SourceCdnError, StorageError } from '@backend/core/errors' +import { + HashLockError, + MediaWikiServerError, + SourceCdnError, + StorageError, +} from '@backend/core/errors' import type { UploadService } from '@backend/db/dal/uploads' import { beforeEach, describe, expect, it, mock } from 'bun:test' import type { Redis } from 'ioredis' @@ -86,6 +91,7 @@ describe('upload worker — retryable errors escape the processor without a DB u ['HashLockError', new HashLockError('lock already held')], ['StorageError', new StorageError('storage write failed')], ['SourceCdnError', new SourceCdnError('cdn returned 503')], + ['MediaWikiServerError', new MediaWikiServerError('MediaWiki upload request failed: 503')], ])('%s propagates out of the processor and leaves the DB untouched', async (_name, error) => { // Force the error to escape early (simulates it bubbling out of the inner try block). mockGetById.mockImplementation(async () => { @@ -112,6 +118,7 @@ describe('upload worker — failed handler resets status to queued on intermedia it.each([ ['HashLockError', new HashLockError('lock already held')], ['SourceCdnError', new SourceCdnError('cdn network error (ECONNRESET)')], + ['MediaWikiServerError', new MediaWikiServerError('MediaWiki upload request failed: 503')], ])('%s: resets status to queued when BullMQ will retry', async (_name, error) => { const failedHandler = capturedHandlers.get('failed') expect(failedHandler).toBeDefined() @@ -141,6 +148,7 @@ describe('upload worker — permanent BullMQ failure marks upload as failed in D it.each([ ['HashLockError', new HashLockError('lock already held')], ['SourceCdnError', new SourceCdnError('cdn returned 503')], + ['MediaWikiServerError', new MediaWikiServerError('MediaWiki upload request failed: 503')], ])('%s: permanently failed job updates DB status to "failed"', async (_name, error) => { const failedHandler = capturedHandlers.get('failed') expect(failedHandler).toBeDefined() diff --git a/backend/src/__tests__/uploadClient.test.ts b/backend/src/__tests__/uploadClient.test.ts index fa50000..8f6d529 100644 --- a/backend/src/__tests__/uploadClient.test.ts +++ b/backend/src/__tests__/uploadClient.test.ts @@ -2,6 +2,7 @@ import { makeRedisMock } from '@backend/__tests__/helpers' import { DuplicateUploadError, HashLockError, + MediaWikiServerError, SourceCdnError, StorageError, } from '@backend/core/errors' @@ -172,6 +173,21 @@ describe('MediaWikiClient.uploadFile error paths', () => { await expect(callUploadFile(client, redis)).rejects.toBeInstanceOf(StorageError) }) + it('throws MediaWikiServerError when the chunk upload endpoint returns 5xx', async () => { + const client = new MediaWikiClient(['key', 'secret']) + // biome-ignore lint/suspicious/noExplicitAny: overriding private methods for testing + ;(client as any).getCsrfToken = mock(async () => 'test-token+\\') + client.findDuplicates = mock(async () => []) + let call = 0 + globalThis.fetch = mock(async () => { + call++ + if (call === 1) return new Response(Buffer.from('data'), { status: 200 }) + return new Response('', { status: 503 }) + }) as unknown as typeof fetch + const { redis } = makeRedisMock() + await expect(callUploadFile(client, redis)).rejects.toBeInstanceOf(MediaWikiServerError) + }) + it('throws HashLockError when lock is already held', async () => { const client = new MediaWikiClient(['key', 'secret']) client.findDuplicates = mock(async () => []) diff --git a/backend/src/core/errors.ts b/backend/src/core/errors.ts index d50218f..565c403 100644 --- a/backend/src/core/errors.ts +++ b/backend/src/core/errors.ts @@ -33,3 +33,10 @@ export class SourceCdnError extends Error { this.name = 'SourceCdnError' } } + +export class MediaWikiServerError extends Error { + constructor(message: string) { + super(message) + this.name = 'MediaWikiServerError' + } +} diff --git a/backend/src/mediawiki/client.ts b/backend/src/mediawiki/client.ts index 9c517ea..d2a7e07 100644 --- a/backend/src/mediawiki/client.ts +++ b/backend/src/mediawiki/client.ts @@ -3,6 +3,7 @@ import type { DuplicateLink } from '@backend/core/errors' import { DuplicateUploadError, HashLockError, + MediaWikiServerError, SourceCdnError, StorageError, } from '@backend/core/errors' @@ -148,7 +149,11 @@ export class MediaWikiClient { }, body: formData, }) - if (!res.ok) throw new Error(`MediaWiki upload request failed: ${res.status}`) + if (!res.ok) { + if (res.status >= 500) + throw new MediaWikiServerError(`MediaWiki upload request failed: ${res.status}`) + throw new Error(`MediaWiki upload request failed: ${res.status}`) + } return res.json() as Promise> } diff --git a/backend/src/workers/upload.worker.ts b/backend/src/workers/upload.worker.ts index 529b4f4..504d09a 100644 --- a/backend/src/workers/upload.worker.ts +++ b/backend/src/workers/upload.worker.ts @@ -3,6 +3,7 @@ import { decryptAccessToken } from '@backend/core/crypto' import { DuplicateUploadError, HashLockError, + MediaWikiServerError, SourceCdnError, StorageError, } from '@backend/core/errors' @@ -230,7 +231,8 @@ export function createUploadWorker(redis: Redis, deps?: WorkerDeps): Worker Date: Sun, 5 Jul 2026 12:30:44 +0200 Subject: [PATCH 2/2] fix: do not auto-retry the final commit request on 5xx Co-Authored-By: Claude Sonnet 5 --- backend/src/__tests__/uploadClient.test.ts | 24 ++++++++++++++++++++++ backend/src/mediawiki/client.ts | 16 ++++++++++++--- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/backend/src/__tests__/uploadClient.test.ts b/backend/src/__tests__/uploadClient.test.ts index 8f6d529..e56574f 100644 --- a/backend/src/__tests__/uploadClient.test.ts +++ b/backend/src/__tests__/uploadClient.test.ts @@ -188,6 +188,30 @@ describe('MediaWikiClient.uploadFile error paths', () => { await expect(callUploadFile(client, redis)).rejects.toBeInstanceOf(MediaWikiServerError) }) + it('throws a plain (non-retryable) Error, not MediaWikiServerError, when the final commit request returns 5xx', async () => { + const client = new MediaWikiClient(['key', 'secret']) + // biome-ignore lint/suspicious/noExplicitAny: overriding private methods for testing + ;(client as any).getCsrfToken = mock(async () => 'test-token+\\') + client.findDuplicates = mock(async () => []) + let call = 0 + globalThis.fetch = mock(async () => { + call++ + if (call === 1) return new Response(Buffer.from('data'), { status: 200 }) // image download + if (call === 2) + return new Response(JSON.stringify({ upload: { filekey: 'stash-key' } }), { status: 200 }) // chunk stash succeeds + return new Response('', { status: 503 }) // final commit fails + }) as unknown as typeof fetch + const { redis } = makeRedisMock() + let caught: unknown + try { + await callUploadFile(client, redis) + } catch (e) { + caught = e + } + expect(caught).toBeInstanceOf(Error) + expect(caught).not.toBeInstanceOf(MediaWikiServerError) + }) + it('throws HashLockError when lock is already held', async () => { const client = new MediaWikiClient(['key', 'secret']) client.findDuplicates = mock(async () => []) diff --git a/backend/src/mediawiki/client.ts b/backend/src/mediawiki/client.ts index d2a7e07..f9a34fd 100644 --- a/backend/src/mediawiki/client.ts +++ b/backend/src/mediawiki/client.ts @@ -131,7 +131,17 @@ export class MediaWikiClient { return titles } - private async apiUploadChunk(formData: FormData): Promise> { + // isFinalCommit distinguishes the terminal commit request from per-chunk stash + // uploads. A chunk-phase 5xx is safe to retry from scratch (the stash isn't a + // public File page yet). A commit-phase 5xx is ambiguous — the page may have + // been created server-side despite the failed response — so retrying the whole + // uploadFile would re-run findDuplicates and could misclassify a real success + // as a duplicate. Only chunk-phase 5xx is surfaced as the retryable + // MediaWikiServerError; commit-phase 5xx stays a terminal Error. + private async apiUploadChunk( + formData: FormData, + isFinalCommit = false, + ): Promise> { const baseParams = { format: 'json', formatversion: '2' } const url = `${config.wikimediaUrls.baseUrl}?${new URLSearchParams(baseParams).toString()}` const authHeader = buildAuthHeader( @@ -150,7 +160,7 @@ export class MediaWikiClient { body: formData, }) if (!res.ok) { - if (res.status >= 500) + if (res.status >= 500 && !isFinalCommit) throw new MediaWikiServerError(`MediaWiki upload request failed: ${res.status}`) throw new Error(`MediaWiki upload request failed: ${res.status}`) } @@ -304,7 +314,7 @@ export class MediaWikiClient { const commitResponse = await withCsrfTokenRetry( `[mw] uploadFile commit (attempt ${attempt + 1})`, () => this.getCsrfToken(), - (t) => this.apiUploadChunk(buildFormData(t)), + (t) => this.apiUploadChunk(buildFormData(t), true), token, ) token = commitResponse.token