mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
fix: say which download client is not responding
Show one notice above the release picker naming the client, with a Try again that resends the same release, instead of a chip on every row. Grab failures carry the client name in errorMeta, and transport errors read "connection refused" rather than "fetch failed".
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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<string, string> = {
|
||||
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}`);
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user