diff --git a/client/src/features/book-requests/__tests__/useReleaseGrab.spec.ts b/client/src/features/book-requests/__tests__/useReleaseGrab.spec.ts index aaa08153c..353e8973d 100644 --- a/client/src/features/book-requests/__tests__/useReleaseGrab.spec.ts +++ b/client/src/features/book-requests/__tests__/useReleaseGrab.spec.ts @@ -191,4 +191,62 @@ describe('useReleaseGrab', () => { expect(picker.isRefused(release({ indexerId: 9, guid: 'torrent-2' }))).toBe(true) expect(picker.isRefused(release({ indexerId: 5, guid: 'usenet-1' }))).toBe(false) }) + + /** "Could not reach qBittorrent: fetch failed" on a toast, then the same chip on every row, was all it said. */ + describe('when the download client is down', () => { + const unavailable: ReleaseGrabOutcome = { + item: null, + reason: 'Could not reach qBittorrent: connection refused', + errorCode: 'GRAB_CLIENT_UNAVAILABLE', + errorMeta: { client: 'QBT' }, + } + + it('names the client once instead of marking every row', async () => { + const picker = makeGrab({ grab: vi.fn<() => Promise>().mockResolvedValue(unavailable) }) + + await picker.handleGrab(release()) + + expect(picker.clientProblem.value).toMatchObject({ code: 'GRAB_CLIENT_UNAVAILABLE', client: 'QBT' }) + expect(picker.isRefused(release({ guid: 'g2' }))).toBe(true) + expect(picker.refusalText(release({ guid: 'g2' }))).toBeNull() + expect(toastMock.error).toHaveBeenCalledWith('bookRequests.errors.grabFailed', { + description: 'bookRequests.releases.clientProblem.unavailable', + }) + }) + + it('sends the same release again without inspecting it again', async () => { + const grab = vi + .fn<() => Promise>() + .mockResolvedValueOnce(unavailable) + .mockResolvedValue({ item: request, reason: null, errorCode: null }) + const picker = makeGrab({ grab }) + await picker.handleGrab(release()) + + await picker.retryClient() + + expect(grab).toHaveBeenNthCalledWith(2, 7, { indexerId: 9, releaseGuid: 'g1' }) + expect(picker.inspectRelease).toHaveBeenCalledTimes(1) + expect(picker.clientProblem.value).toBeNull() + expect(picker.isRefused(release({ guid: 'g2' }))).toBe(false) + }) + + it('keeps the notice when the client still does not answer', async () => { + const picker = makeGrab({ grab: vi.fn<() => Promise>().mockResolvedValue(unavailable) }) + await picker.handleGrab(release()) + + await picker.retryClient() + + expect(picker.clientProblem.value).toMatchObject({ code: 'GRAB_CLIENT_UNAVAILABLE' }) + expect(picker.isRefused(release({ guid: 'g2' }))).toBe(true) + }) + + it('is cleared by a fresh search', async () => { + const picker = makeGrab({ grab: vi.fn<() => Promise>().mockResolvedValue(unavailable) }) + await picker.handleGrab(release()) + + picker.forgetRefusals() + + expect(picker.clientProblem.value).toBeNull() + }) + }) }) diff --git a/client/src/features/book-requests/components/ReleasePickerPanel.vue b/client/src/features/book-requests/components/ReleasePickerPanel.vue index 2f6bd30ad..f1a44a987 100644 --- a/client/src/features/book-requests/components/ReleasePickerPanel.vue +++ b/client/src/features/book-requests/components/ReleasePickerPanel.vue @@ -7,6 +7,7 @@ import { Check, ChevronDown, ChevronUp, + CircleAlert, CircleCheck, CircleMinus, Download, @@ -179,17 +180,29 @@ const { resetSearchEditorState, } = useReleaseSearchEditor({ request, criteria, attempts, requestId, fetchReleases, beforeSearch: startFreshSearch }) -const { manualOpen, grabbing, holdForReview, openManual, closeManual, forgetRefusals, handleGrab, handleManualGrab, isRefused, refusalText } = - useReleaseGrab({ - request, - requestId, - grab: actions.grab, - setRequest, - inspectRelease, - setFilesExpanded, - deliveryFor, - afterGrab, - }) +const { + manualOpen, + grabbing, + holdForReview, + clientProblem, + openManual, + closeManual, + forgetRefusals, + handleGrab, + handleManualGrab, + retryClient, + isRefused, + refusalText, +} = useReleaseGrab({ + request, + requestId, + grab: actions.grab, + setRequest, + inspectRelease, + setFilesExpanded, + deliveryFor, + afterGrab, +}) /** * Every Send is off while one release is on its way, not only the row being inspected. The @@ -197,6 +210,20 @@ const { manualOpen, grabbing, holdForReview, openManual, closeManual, forgetRefu * request, so `actions` has nothing to be busy about yet. */ const busy = computed(() => grabbing.value || actions.isPending(request.value?.id)) +const clientProblemTitle = computed(() => { + const problem = clientProblem.value + if (!problem) return '' + const kind = problem.code === 'GRAB_CLIENT_UNAVAILABLE' ? 'unavailable' : 'refused' + return problem.client + ? t(`bookRequests.releases.clientProblem.${kind}`, { client: problem.client }) + : t(`bookRequests.releases.clientProblem.${kind}Unnamed`) +}) +const clientProblemHint = computed(() => { + const problem = clientProblem.value + if (!problem) return '' + if (problem.code === 'GRAB_CLIENT_UNAVAILABLE') return t('bookRequests.releases.clientProblem.unavailableHint') + return problem.reason ?? t('bookRequests.releases.clientProblem.refusedHint') +}) /** A retry is the same picker; only the framing changes, so the approver knows why it reopened. */ const isRetry = computed(() => request.value?.status === 'failed') const failureText = computed(() => (request.value ? requestFailureText(request.value, (key, named) => t(key, named)) : null)) @@ -733,6 +760,10 @@ function startFreshSearch(): void { forgetRefusals() } +function handleRetryClient() { + void retryClient() +} + function handleRefresh() { startFreshSearch() if (!Number.isInteger(requestId.value)) return @@ -1138,6 +1169,26 @@ function showEverySource(): void { {{ t('bookRequests.releases.cached') }}

+ + +
diff --git a/client/src/features/book-requests/composables/useReleaseGrab.ts b/client/src/features/book-requests/composables/useReleaseGrab.ts index a356a4920..3935e10c8 100644 --- a/client/src/features/book-requests/composables/useReleaseGrab.ts +++ b/client/src/features/book-requests/composables/useReleaseGrab.ts @@ -4,6 +4,7 @@ import { useRoute, useRouter } from 'vue-router' import { toast } from 'vue-sonner' import { findGrabRefusal, releaseInspectionBlocksGrab } from '@bookorbit/types' import type { + BookRequestFailureMeta, BookRequestItem, DownloadDelivery, GrabBookRequestPayload, @@ -13,13 +14,31 @@ import type { ReleaseFileInspection, } from '@bookorbit/types' -/** Only the refusals that rule another release out get a note; the rest are about one row. */ +/** + * Only the refusals that rule another release out get a note; the rest are about one row. A client + * refusal is not about any row, so it is said once above the list instead of on every one. + */ const REFUSAL_KEYS: Partial> = { GRAB_SOURCE_REFUSED: 'sourceRefused', GRAB_SOURCE_UNAVAILABLE: 'sourceUnavailable', GRAB_VIP_REQUIRED: 'vipRequired', - GRAB_CLIENT_REFUSED: 'clientRefused', - GRAB_CLIENT_UNAVAILABLE: 'clientUnavailable', +} + +type ClientFailureCode = Extract + +function isClientFailure(code: GrabFailureCode | null): code is ClientFailureCode { + return code === 'GRAB_CLIENT_REFUSED' || code === 'GRAB_CLIENT_UNAVAILABLE' +} + +/** The download client that failed the last send, and that send, so it can be tried once more. */ +export interface ClientProblem { + code: ClientFailureCode + /** The client's configured name, when the server said which one. */ + client: string | null + /** The client's own words. Only shown for a refusal: "not responding" is all an outage needs. */ + reason: string | null + body: GrabBookRequestPayload + delivery: DownloadDelivery } /** The refusal travels back with the call it belongs to, so one grab cannot describe another. */ @@ -27,6 +46,7 @@ export interface ReleaseGrabOutcome { item: BookRequestItem | null reason: string | null errorCode: GrabFailureCode | null + errorMeta?: BookRequestFailureMeta | null } export interface ReleaseGrabOptions { @@ -57,6 +77,7 @@ export function useReleaseGrab(options: ReleaseGrabOptions) { const manualOpen = ref(false) const refusals = ref([]) + const clientProblem = ref(null) /** * One release at a time, from the click rather than from the request. * @@ -85,6 +106,16 @@ export function useReleaseGrab(options: ReleaseGrabOptions) { /** A fresh search is also how an approver retries a source that was merely slow. */ function forgetRefusals() { refusals.value = [] + clientProblem.value = null + } + + function clientName(meta: BookRequestFailureMeta | null | undefined): string | null { + const name = meta?.client + return typeof name === 'string' && name.trim() ? name : null + } + + function clientUnavailableText(client: string | null): string { + return client ? t('bookRequests.releases.clientProblem.unavailable', { client }) : t('bookRequests.releases.clientProblem.unavailableUnnamed') } /** A sent release leaves nothing to pick, so the drawer pops back to the request it belongs to, unless told otherwise. */ @@ -93,9 +124,18 @@ export function useReleaseGrab(options: ReleaseGrabOptions) { const outcome = await grab(request.value.id, holdForReview.value ? { ...body, holdForReview: true } : body) if (!outcome.item) { rememberRefusal(body, delivery, outcome.errorCode) + const code = outcome.errorCode + if (isClientFailure(code)) { + const client = clientName(outcome.errorMeta) + clientProblem.value = { code, client, reason: outcome.reason, body, delivery } + const description = code === 'GRAB_CLIENT_UNAVAILABLE' ? clientUnavailableText(client) : outcome.reason + toast.error(t('bookRequests.errors.grabFailed'), description ? { description } : undefined) + return + } toast.error(t('bookRequests.errors.grabFailed'), outcome.reason ? { description: outcome.reason } : undefined) return } + clientProblem.value = null setRequest(outcome.item) manualOpen.value = false toast.success(t('bookRequests.toasts.grabbed')) @@ -136,6 +176,23 @@ export function useReleaseGrab(options: ReleaseGrabOptions) { void submitGrab(payload.body) } + /** + * The same release again, once the client is back. Re-searching was the only way out before, + * which asked every tracker again to undo something none of them had said. + */ + async function retryClient(): Promise { + const problem = clientProblem.value + if (!problem || grabbing.value) return + refusals.value = refusals.value.filter((refusal) => !isClientFailure(refusal.code)) + clientProblem.value = null + grabbing.value = true + try { + await sendGrab(problem.body, problem.delivery) + } finally { + grabbing.value = false + } + } + /** Only a refusal the server classified; anything else says nothing about the other releases. */ function rememberRefusal(body: GrabBookRequestPayload, delivery: DownloadDelivery, code: GrabFailureCode | null) { if (!code) return @@ -161,12 +218,14 @@ export function useReleaseGrab(options: ReleaseGrabOptions) { grabbing, holdForReview, refusals, + clientProblem, openManual, closeManual, forgetRefusals, submitGrab, handleGrab, handleManualGrab, + retryClient, isRefused, refusalText, } diff --git a/client/src/locales/en.json b/client/src/locales/en.json index d846552ee..64392122c 100644 --- a/client/src/locales/en.json +++ b/client/src/locales/en.json @@ -8869,6 +8869,15 @@ "clientRefused": "Your download client refused an earlier release", "clientUnavailable": "Your download client did not answer an earlier release" }, + "clientProblem": { + "unavailable": "{client} is not responding", + "unavailableUnnamed": "Your download client is not responding", + "unavailableHint": "Releases that use it can't be sent until it answers. Check that it is running, then try again.", + "refused": "{client} turned down the release", + "refusedUnnamed": "Your download client turned down the release", + "refusedHint": "Releases that use it can't be sent until this is fixed.", + "retry": "Try again" + }, "singleFileNote": "A release can be one book in several formats, an audiobook in parts, or an archive - all of those import as a single book. A release holding several separate books asks you which one to keep.", "holdForReview": "Review the download before it is filed", "releasePageLabel": "{title}, release page on {indexer}", diff --git a/server/src/modules/book-request/download-clients/adapters/client-http.utils.test.ts b/server/src/modules/book-request/download-clients/adapters/client-http.utils.test.ts index b8fb60970..788fd72e8 100644 --- a/server/src/modules/book-request/download-clients/adapters/client-http.utils.test.ts +++ b/server/src/modules/book-request/download-clients/adapters/client-http.utils.test.ts @@ -98,6 +98,26 @@ describe('fetchClient', () => { }); }); + it('says why the connection failed instead of "fetch failed"', async () => { + const refused = Object.assign(new Error('connect ECONNREFUSED 127.0.0.1:8082'), { code: 'ECONNREFUSED' }); + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new TypeError('fetch failed', { cause: refused }))); + + await expect(fetchClient(url, {}, 'qBittorrent')).rejects.toMatchObject({ message: 'Could not reach qBittorrent: connection refused' }); + }); + + it('reads the cause from an aggregate of several addresses', async () => { + const cause = new AggregateError([Object.assign(new Error('connect EHOSTUNREACH'), { code: 'EHOSTUNREACH' })]); + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new TypeError('fetch failed', { cause }))); + + await expect(fetchClient(url, {}, 'Deluge')).rejects.toMatchObject({ message: 'Could not reach Deluge: host unreachable' }); + }); + + it('keeps the original message when the cause is not one it knows', async () => { + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new TypeError('fetch failed'))); + + await expect(fetchClient(url, {}, 'NZBGet')).rejects.toMatchObject({ message: 'Could not reach NZBGet: fetch failed' }); + }); + /** * `fetch` reports a timeout and a refusal as the same opaque `TypeError`, so the abort has to be * told apart from every other failure here or the operator is handed "fetch failed" either way. diff --git a/server/src/modules/book-request/download-clients/adapters/client-http.utils.ts b/server/src/modules/book-request/download-clients/adapters/client-http.utils.ts index 6d98f2221..35d7d3859 100644 --- a/server/src/modules/book-request/download-clients/adapters/client-http.utils.ts +++ b/server/src/modules/book-request/download-clients/adapters/client-http.utils.ts @@ -47,12 +47,35 @@ export async function fetchClient(url: URL, init: RequestInit, label: string, sa if (error instanceof Error && error.name === 'AbortError') { throw new ServiceUnavailableException(`${label} did not answer in time`); } - throw new ServiceUnavailableException(`Could not reach ${label}: ${error instanceof Error ? error.message : String(error)}`); + throw new ServiceUnavailableException(`Could not reach ${label}: ${transportFailureText(error)}`); } finally { clearTimeout(timer); } } +const TRANSPORT_FAILURES: Record = { + ECONNREFUSED: 'connection refused', + ECONNRESET: 'connection reset', + ENOTFOUND: 'host not found', + EAI_AGAIN: 'host not found', + EHOSTUNREACH: 'host unreachable', + ENETUNREACH: 'network unreachable', + ETIMEDOUT: 'connection timed out', + UND_ERR_CONNECT_TIMEOUT: 'connection timed out', +}; + +/** + * Undici reports every transport failure as `TypeError: fetch failed` and keeps the reason on + * `cause`, sometimes inside an `AggregateError` when several addresses were tried. "fetch failed" + * is what the attempt history showed for a client that was simply not running. + */ +function transportFailureText(error: unknown): string { + const message = error instanceof Error ? error.message : String(error); + const cause = error instanceof Error ? (error.cause as { code?: unknown; errors?: { code?: unknown }[] } | undefined) : undefined; + const code = cause?.code ?? cause?.errors?.[0]?.code; + return (typeof code === 'string' && TRANSPORT_FAILURES[code]) || message; +} + /** Daemon-side failures are temporary; authentication and request refusals remain durable 4xx. */ export function throwForClientServerError(response: Response, label: string, operation: string): void { if (response.status >= 500) throw new ServiceUnavailableException(`${label} answered ${response.status} for ${operation}`); diff --git a/server/src/modules/book-request/fulfillment/request-fulfillment.service.test.ts b/server/src/modules/book-request/fulfillment/request-fulfillment.service.test.ts index d6c2fa84c..77863c28a 100644 --- a/server/src/modules/book-request/fulfillment/request-fulfillment.service.test.ts +++ b/server/src/modules/book-request/fulfillment/request-fulfillment.service.test.ts @@ -228,7 +228,7 @@ describe('RequestFulfillmentService.grab', () => { await expect(service.grab(7, { magnet: MAGNET }, user())).rejects.toMatchObject({ status: 503, - response: { errorCode: 'GRAB_CLIENT_UNAVAILABLE' }, + response: { errorCode: 'GRAB_CLIENT_UNAVAILABLE', errorMeta: { client: 'qbit' } }, }); expect(adapter.add).toHaveBeenCalledTimes(1); diff --git a/server/src/modules/book-request/fulfillment/request-fulfillment.service.ts b/server/src/modules/book-request/fulfillment/request-fulfillment.service.ts index 1c4d13ce4..86dada191 100644 --- a/server/src/modules/book-request/fulfillment/request-fulfillment.service.ts +++ b/server/src/modules/book-request/fulfillment/request-fulfillment.service.ts @@ -323,10 +323,12 @@ export class RequestFulfillmentService { `[book_request.grab] [fail] requestId=${requestId} downloadId=${download.id} clientId=${client?.id ?? 'direct'} error="${sanitizeLogValue(message)}" - the download could not be started`, ); // Which half refused decides what another attempt should do: a client that is down refuses - // every torrent alike, while a file BookOrbit fetches itself was refused by the source. + // every torrent alike, while a file BookOrbit fetches itself was refused by the source. The + // client's configured name lets the picker say which one, in the reader's language. throw withGrabCode( error, client === null ? 'GRAB_SOURCE_REFUSED' : isServiceUnavailable(error) ? 'GRAB_CLIENT_UNAVAILABLE' : 'GRAB_CLIENT_REFUSED', + client === null ? undefined : { client: client.name }, ); } @@ -784,7 +786,7 @@ export class RequestFulfillmentService { } const client = await this.clients.findOne(requestedId); - if (!client.enabled) throw grabError('GRAB_CLIENT_REFUSED', `Download client "${client.name}" is disabled`); + if (!client.enabled) throw grabError('GRAB_CLIENT_REFUSED', `Download client "${client.name}" is disabled`, { client: client.name }); this.assertClientCanDeliver(client, source); this.assertClientCanImport(client); return client; @@ -883,16 +885,16 @@ function errorCodeOf(error: unknown): string { return typeof code === 'string' ? code : 'UNKNOWN'; } -function grabError(errorCode: GrabFailureCode, message: string): BadRequestException { - return new BadRequestException({ message, errorCode, statusCode: HttpStatus.BAD_REQUEST }); +function grabError(errorCode: GrabFailureCode, message: string, errorMeta?: BookRequestFailureMeta): BadRequestException { + return new BadRequestException({ message, errorCode, ...(errorMeta ? { errorMeta } : {}), statusCode: HttpStatus.BAD_REQUEST }); } -function grabUnavailable(errorCode: GrabFailureCode, message: string): ServiceUnavailableException { - return new ServiceUnavailableException({ message, errorCode, statusCode: HttpStatus.SERVICE_UNAVAILABLE }); +function grabUnavailable(errorCode: GrabFailureCode, message: string, errorMeta?: BookRequestFailureMeta): ServiceUnavailableException { + return new ServiceUnavailableException({ message, errorCode, ...(errorMeta ? { errorMeta } : {}), statusCode: HttpStatus.SERVICE_UNAVAILABLE }); } -function grabConflict(errorCode: GrabFailureCode, message: string): ConflictException { - return new ConflictException({ message, errorCode, statusCode: HttpStatus.CONFLICT }); +function grabConflict(errorCode: GrabFailureCode, message: string, errorMeta?: BookRequestFailureMeta): ConflictException { + return new ConflictException({ message, errorCode, ...(errorMeta ? { errorMeta } : {}), statusCode: HttpStatus.CONFLICT }); } /** @@ -914,14 +916,14 @@ export class RequestAlreadyClaimedException extends ConflictException { * Anything that is not an HTTP failure is handed back untouched: an unexpected error is a fault * on our side, and dressing it as a refusal would let the failover route around a real bug. */ -function withGrabCode(error: unknown, errorCode: GrabFailureCode): unknown { +function withGrabCode(error: unknown, errorCode: GrabFailureCode, errorMeta?: BookRequestFailureMeta): unknown { if (!(error instanceof HttpException)) return error; const status = error.getStatus(); - if (status === (HttpStatus.SERVICE_UNAVAILABLE as number)) return grabUnavailable(errorCode, error.message); - if (status === (HttpStatus.CONFLICT as number)) return grabConflict(errorCode, error.message); - if (status === (HttpStatus.BAD_REQUEST as number)) return grabError(errorCode, error.message); - return new HttpException({ message: error.message, errorCode, statusCode: status }, status); + if (status === (HttpStatus.SERVICE_UNAVAILABLE as number)) return grabUnavailable(errorCode, error.message, errorMeta); + if (status === (HttpStatus.CONFLICT as number)) return grabConflict(errorCode, error.message, errorMeta); + if (status === (HttpStatus.BAD_REQUEST as number)) return grabError(errorCode, error.message, errorMeta); + return new HttpException({ message: error.message, errorCode, ...(errorMeta ? { errorMeta } : {}), statusCode: status }, status); } function isServiceUnavailable(error: unknown): boolean {