Skip to content

Commit 9f251ca

Browse files
committed
feat(tasks): enhance FetchUrlJobError to include detailed HTTP error messages
- Added `httpErrorMessage` field to `FetchUrlJobErrorDetails` and `FetchUrlJobErrorInstance` for better error reporting. - Updated `createFetchUrlHttpError` to construct messages that include JSON error details from the response body. - Introduced `jsonMessageFromHttpBody` function to extract meaningful error messages from JSON responses. - Modified `buildHttpError` to read and pass the error body to the error creation function. - Added a test case to verify that JSON error messages are correctly surfaced in the `FetchUrlTask` error handling.
1 parent ccd1a05 commit 9f251ca

3 files changed

Lines changed: 78 additions & 5 deletions

File tree

packages/tasks/src/task/FetchUrlJobError.ts

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,13 +78,15 @@ export interface FetchUrlJobErrorDetails {
7878
readonly url?: string;
7979
readonly httpStatus?: number;
8080
readonly httpStatusText?: string;
81+
readonly httpErrorMessage?: string;
8182
}
8283

8384
export type FetchUrlJobErrorInstance = JobError & {
8485
code: FetchUrlErrorCodeValue;
8586
url?: string;
8687
httpStatus?: number;
8788
httpStatusText?: string;
89+
httpErrorMessage?: string;
8890
retryDate?: Date;
8991
};
9092

@@ -104,6 +106,9 @@ function attachFetchUrlFields(
104106
if (details?.httpStatusText !== undefined) {
105107
withCode.httpStatusText = details.httpStatusText;
106108
}
109+
if (details?.httpErrorMessage !== undefined) {
110+
withCode.httpErrorMessage = details.httpErrorMessage;
111+
}
107112
return withCode;
108113
}
109114

@@ -177,18 +182,40 @@ export function createFetchUrlHttpError(
177182
url: string,
178183
status: number,
179184
statusText: string,
180-
retryDate?: Date
185+
retryDate?: Date,
186+
body?: string
181187
): FetchUrlJobErrorInstance {
182188
const code = httpStatusToFetchUrlErrorCode(status);
183-
const message = `Failed to fetch ${url}: ${status} ${statusText}`;
189+
const httpErrorMessage = jsonMessageFromHttpBody(body);
190+
const statusPart = `${status} ${statusText}`;
191+
const message =
192+
httpErrorMessage !== undefined
193+
? `Failed to fetch ${url}: ${statusPart}: ${httpErrorMessage}`
194+
: `Failed to fetch ${url}: ${statusPart}`;
184195
return createFetchUrlJobError(code, message, {
185196
url,
186197
httpStatus: status,
187198
httpStatusText: statusText,
199+
httpErrorMessage,
188200
retryDate,
189201
});
190202
}
191203

204+
/** Reads `{message}` from a JSON error body, if that field is a non-empty string. */
205+
export function jsonMessageFromHttpBody(body: string | undefined): string | undefined {
206+
if (body === undefined || body.trim() === "") return undefined;
207+
try {
208+
const parsed: unknown = JSON.parse(body);
209+
if (parsed === null || typeof parsed !== "object") return undefined;
210+
const message = (parsed as { message?: unknown }).message;
211+
if (typeof message !== "string") return undefined;
212+
const trimmed = message.trim();
213+
return trimmed.length > 0 ? trimmed : undefined;
214+
} catch {
215+
return undefined;
216+
}
217+
}
218+
192219
/**
193220
* True when `error` (or a nested `cause`) is a dropped connection / DNS /
194221
* timeout rather than a completed body we failed to decode. `response.text()`

packages/tasks/src/task/FetchUrlTask.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ function assertResponseType(
411411
);
412412
}
413413

414-
function buildHttpError(url: string, response: Response): Error {
414+
async function buildHttpError(url: string, response: Response): Promise<Error> {
415415
let retryDate: Date | undefined;
416416
if (response.status === 429 || response.status === 503 || response.headers.get("Retry-After")) {
417417
const retryAfterStr = response.headers.get("Retry-After");
@@ -425,7 +425,22 @@ function buildHttpError(url: string, response: Response): Error {
425425
}
426426
}
427427
}
428-
return createFetchUrlHttpError(url, response.status, response.statusText, retryDate);
428+
const body = await readHttpErrorBody(response);
429+
return createFetchUrlHttpError(url, response.status, response.statusText, retryDate, body);
430+
}
431+
432+
const HTTP_ERROR_BODY_MAX_BYTES = 4096;
433+
434+
async function readHttpErrorBody(response: Response): Promise<string | undefined> {
435+
try {
436+
const text = await response.text();
437+
if (text.length === 0) return undefined;
438+
return text.length > HTTP_ERROR_BODY_MAX_BYTES
439+
? text.slice(0, HTTP_ERROR_BODY_MAX_BYTES)
440+
: text;
441+
} catch {
442+
return undefined;
443+
}
429444
}
430445

431446
/**
@@ -593,8 +608,9 @@ export class FetchUrlJob<
593608
}
594609

595610
if (!response.ok) {
611+
const error = await buildHttpError(input.url!, response);
596612
await discardBody(response);
597-
throw buildHttpError(input.url!, response);
613+
throw error;
598614
}
599615

600616
if ((input.method ?? "GET").toUpperCase() === "HEAD") {

packages/test/src/test/task/FetchTask.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,36 @@ describe("FetchUrlTask", () => {
231231
expect(mockFetch.mock.calls.length).toBe(1);
232232
});
233233

234+
test("surfaces JSON message from a non-2xx body", async () => {
235+
mockFetch.mockImplementation(() =>
236+
Promise.resolve(
237+
new Response(
238+
JSON.stringify({
239+
error: "Internal server error",
240+
message: "upstream rejected the request",
241+
}),
242+
{
243+
status: 500,
244+
statusText: "Internal Server Error",
245+
headers: { "Content-Type": "application/json" },
246+
}
247+
)
248+
)
249+
);
250+
251+
const error = await fetchUrl({
252+
url: "https://api.example.com/items",
253+
response_type: "json",
254+
}).catch((e: unknown) => e);
255+
expect(error).toBeInstanceOf(JobTaskFailedError);
256+
const jobFailed = error as JobTaskFailedError;
257+
expect(jobFailed.jobError.message).toContain("upstream rejected the request");
258+
expect(isFetchUrlJobError(jobFailed.jobError)).toBe(true);
259+
if (isFetchUrlJobError(jobFailed.jobError)) {
260+
expect(jobFailed.jobError.httpErrorMessage).toBe("upstream rejected the request");
261+
}
262+
});
263+
234264
test("handles network errors", async () => {
235265
mockFetch.mockImplementation(() => Promise.reject(new Error("Network error")));
236266

0 commit comments

Comments
 (0)