mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-11-15 18:28:13 +00:00
fix(search): name the proxy when a gateway cuts a search off (#1522)
A reverse proxy whose read timeout is shorter than the release search budget (nginx defaults to 60s) answers a slow search with its own 504 page. describeSearchFailure found no server message there and fell back to "Unable to reach download source. Network may be restricted or mirrors blocked.", which sent users off changing mirrors (discussion #1520). - 504 and Cloudflare's 524 now say the proxy stopped waiting and that its read timeout should be at least the release search timeout. - A 502/503 with a non-JSON body keeps fetchJSON's "Server unavailable" wording instead of the mirrors line. - docs: the root-path nginx example sets proxy_read_timeout and proxy_send_timeout like the subpath ones, and a new troubleshooting section covers searches that time out behind a proxy.
This commit is contained in:
1 parent
eab8f7bc9a
commit
d887942630
3 files changed
+73
-2
No files matched your search
@@ -4,6 +4,7 @@ import { searchBooks } from '../services/api';
|
||||
import {
|
||||
describeSearchFailure,
|
||||
CLIENT_TIMEOUT_MESSAGE,
|
||||
PROXY_TIMEOUT_MESSAGE,
|
||||
UNREACHABLE_SOURCE_MESSAGE,
|
||||
} from '../utils/searchFailureMessage';
|
||||
|
||||
@@ -22,6 +23,14 @@ const jsonResponse = (body: unknown, status: number): Response =>
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
});
|
||||
|
||||
/** A reverse proxy's own error page: HTML, so there is no JSON for the client to read. */
|
||||
const proxyErrorPage = (status: number, statusText: string): Response =>
|
||||
new Response(`<html><body><h1>${status} ${statusText}</h1></body></html>`, {
|
||||
status,
|
||||
statusText,
|
||||
headers: { 'Content-Type': 'text/html' },
|
||||
});
|
||||
|
||||
/** Drive a real searchBooks() failure so the error is the one the hook actually sees. */
|
||||
const failedSearch = async (respond: () => Promise<Response>): Promise<unknown> => {
|
||||
vi.stubGlobal('fetch', vi.fn(respond));
|
||||
@@ -51,6 +60,30 @@ describe('describeSearchFailure', () => {
|
||||
expect(describeSearchFailure(error)).not.toContain('mirrors');
|
||||
});
|
||||
|
||||
it("blames the proxy's read timeout when a gateway gave up on the search", async () => {
|
||||
// Discussion #1520: nginx's 60s default cut off every search that needed a
|
||||
// protection solve, and the mirrors line sent the user off changing mirrors.
|
||||
const error = await failedSearch(() =>
|
||||
Promise.resolve(proxyErrorPage(504, 'Gateway Time-out')),
|
||||
);
|
||||
|
||||
expect(describeSearchFailure(error)).toBe(PROXY_TIMEOUT_MESSAGE);
|
||||
expect(describeSearchFailure(error)).not.toContain('mirrors');
|
||||
});
|
||||
|
||||
it("treats Cloudflare's origin timeout as the proxy's too", async () => {
|
||||
const error = await failedSearch(() => Promise.resolve(proxyErrorPage(524, '')));
|
||||
|
||||
expect(describeSearchFailure(error)).toBe(PROXY_TIMEOUT_MESSAGE);
|
||||
});
|
||||
|
||||
it("keeps the gateway's own wording when the proxy could not reach Shelfmark", async () => {
|
||||
const error = await failedSearch(() => Promise.resolve(proxyErrorPage(502, 'Bad Gateway')));
|
||||
|
||||
expect(describeSearchFailure(error)).toContain('Server unavailable (502)');
|
||||
expect(describeSearchFailure(error)).not.toBe(UNREACHABLE_SOURCE_MESSAGE);
|
||||
});
|
||||
|
||||
it('falls back to the mirrors line only when nothing explained itself', async () => {
|
||||
const error = await failedSearch(() => Promise.resolve(jsonResponse({}, 503)));
|
||||
|
||||
|
||||
@@ -12,14 +12,29 @@ export const CLIENT_TIMEOUT_MESSAGE =
|
||||
'The search took longer than the server said it would. Raise the release search ' +
|
||||
'timeout if your setup is simply slow.';
|
||||
|
||||
// Shown when a gateway in front of Shelfmark gave up waiting on the search. Shelfmark
|
||||
// never sends these statuses itself - a search it cannot finish ends in a 503 that says
|
||||
// why - so the proxy's read timeout is shorter than the search budget. See discussion
|
||||
// #1520, where nginx's 60s default cut off every search that needed a protection solve.
|
||||
export const PROXY_TIMEOUT_MESSAGE =
|
||||
'A reverse proxy in front of Shelfmark stopped waiting before the search finished. ' +
|
||||
"Raise the proxy's read timeout to at least the release search timeout.";
|
||||
|
||||
// 504 Gateway Timeout, and Cloudflare's 524 for an origin that answered too slowly.
|
||||
const PROXY_TIMEOUT_STATUSES = new Set([504, 524]);
|
||||
|
||||
// Statuses fetchJSON already words as the server or its proxy being unavailable.
|
||||
const GATEWAY_UNAVAILABLE_STATUSES = new Set([502, 503]);
|
||||
|
||||
/**
|
||||
* The sentence to show for a failed direct-mode search.
|
||||
*
|
||||
* The server knows why a search failed - a spent search budget, an unsolved protection
|
||||
* challenge - and blanket-replacing that with the mirrors line told users their network
|
||||
* was broken when it was not. So: the server's own words when it explained itself, the
|
||||
* timeout line when we gave up before it answered, and the mirrors line only when
|
||||
* neither applies. See issue #1285.
|
||||
* timeout line when we gave up before it answered, the proxy's part when a gateway
|
||||
* answered instead of Shelfmark, and the mirrors line only when none of those applies.
|
||||
* See issue #1285 and discussion #1520.
|
||||
*/
|
||||
export const describeSearchFailure = (error: unknown): string => {
|
||||
if (isTimeoutError(error)) {
|
||||
@@ -30,6 +45,17 @@ export const describeSearchFailure = (error: unknown): string => {
|
||||
return error.serverMessage;
|
||||
}
|
||||
|
||||
if (isApiResponseError(error)) {
|
||||
if (PROXY_TIMEOUT_STATUSES.has(error.status)) {
|
||||
return PROXY_TIMEOUT_MESSAGE;
|
||||
}
|
||||
// A 502/503 without a JSON body is a proxy's error page, not Shelfmark's API: the
|
||||
// upstream is down or restarting, which says nothing about mirrors either.
|
||||
if (GATEWAY_UNAVAILABLE_STATUSES.has(error.status) && !error.payload) {
|
||||
return error.message;
|
||||
}
|
||||
}
|
||||
|
||||
const message = error instanceof Error ? error.message : '';
|
||||
if (message.includes('Network restricted') || message.includes('Unable to reach')) {
|
||||
return message;
|
||||
|
||||
Reference in new issue
Block a user