From 0ca7c030da30e5aa4ea55f7735c812208acf4542 Mon Sep 17 00:00:00 2001 From: Jacob Lee Date: Tue, 17 Sep 2019 14:03:28 -0500 Subject: [PATCH 1/3] Make sure useFetch rejects with an Error type. Previously, a non-ok http response would reject with the response object. It's better for rejections to be of type Error so that the full stack trace information is available; plus, the TypeScript type definition assumes that the error object is always instanceof Error. Instead, failed responses reject with a FetchError, with the underlying Response object available as error.response. This is a backward-incompatible change: users who expected `error` to be of type Response now have to refer to `error.response` instead. --- packages/react-async/src/index.d.ts | 4 ++++ packages/react-async/src/index.js | 2 +- packages/react-async/src/useAsync.js | 9 ++++++++- packages/react-async/src/useAsync.spec.js | 22 +++++++++++++++++++--- 4 files changed, 32 insertions(+), 5 deletions(-) diff --git a/packages/react-async/src/index.d.ts b/packages/react-async/src/index.d.ts index 4d46575e..eee2d9ff 100644 --- a/packages/react-async/src/index.d.ts +++ b/packages/react-async/src/index.d.ts @@ -234,4 +234,8 @@ type FetchRun = { run(): void } +export interface FetchError extends Error { + response: Response +} + export default Async diff --git a/packages/react-async/src/index.js b/packages/react-async/src/index.js index 5c8aba24..e9409c85 100644 --- a/packages/react-async/src/index.js +++ b/packages/react-async/src/index.js @@ -1,6 +1,6 @@ import Async from "./Async" export { default as Async, createInstance } from "./Async" -export { default as useAsync, useFetch } from "./useAsync" +export { default as useAsync, useFetch, FetchError } from "./useAsync" export default Async export { statusTypes } from "./status" export { default as globalScope } from "./globalScope" diff --git a/packages/react-async/src/useAsync.js b/packages/react-async/src/useAsync.js index b2c14509..420bbf42 100644 --- a/packages/react-async/src/useAsync.js +++ b/packages/react-async/src/useAsync.js @@ -154,8 +154,15 @@ const useAsync = (arg1, arg2) => { ) } +export class FetchError extends Error { + constructor(response) { + super(response.statusText) + this.response = response + } +} + const parseResponse = (accept, json) => res => { - if (!res.ok) return Promise.reject(res) + if (!res.ok) return Promise.reject(new FetchError(res)) if (typeof json === "boolean") return json ? res.json() : res return accept === "application/json" ? res.json() : res } diff --git a/packages/react-async/src/useAsync.spec.js b/packages/react-async/src/useAsync.spec.js index b6501578..af14be26 100644 --- a/packages/react-async/src/useAsync.spec.js +++ b/packages/react-async/src/useAsync.spec.js @@ -3,7 +3,7 @@ import "@testing-library/jest-dom/extend-expect" import React from "react" import { render, fireEvent, cleanup } from "@testing-library/react" -import { useAsync, useFetch, globalScope } from "./index" +import { useAsync, useFetch, globalScope, FetchError } from "./index" import { sleep, resolveTo, @@ -20,11 +20,11 @@ const abortCtrl = { abort: jest.fn(), signal: "SIGNAL" } globalScope.AbortController = jest.fn(() => abortCtrl) const json = jest.fn(() => ({})) -globalScope.fetch = jest.fn(() => Promise.resolve({ ok: true, json })) +globalScope.fetch = jest.fn() beforeEach(abortCtrl.abort.mockClear) -beforeEach(globalScope.fetch.mockClear) beforeEach(json.mockClear) +beforeEach(() => globalScope.fetch.mockReset().mockResolvedValue({ ok: true, json })) afterEach(cleanup) const Async = ({ children = () => null, ...props }) => children(useAsync(props)) @@ -250,4 +250,20 @@ describe("useFetch", () => { expect.objectContaining({ preventDefault: expect.any(Function) }) ) }) + + test("throws a FetchError for failed requests", async () => { + const errorResponse = { ok: false, statusText: "Bad Request", json } + globalScope.fetch.mockResolvedValue(errorResponse) + const onResolve = jest.fn() + const onReject = jest.fn() + render() + expect(globalScope.fetch).toHaveBeenCalled() + await sleep(10) + expect(onResolve).not.toHaveBeenCalled() + expect(onReject).toHaveBeenCalled() + let [err] = onReject.mock.calls[0] + expect(err).toBeInstanceOf(FetchError) + expect(err.message).toEqual("Bad Request") + expect(err.response).toBe(errorResponse) + }) }) From fcc67d2e432c7771ce577ed759c18bf1885bb155 Mon Sep 17 00:00:00 2001 From: Jacob Lee Date: Thu, 19 Sep 2019 11:12:12 -0500 Subject: [PATCH 2/3] FetchError: add status code to the error message. --- packages/react-async/src/useAsync.js | 2 +- packages/react-async/src/useAsync.spec.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react-async/src/useAsync.js b/packages/react-async/src/useAsync.js index 420bbf42..1490a6b8 100644 --- a/packages/react-async/src/useAsync.js +++ b/packages/react-async/src/useAsync.js @@ -156,7 +156,7 @@ const useAsync = (arg1, arg2) => { export class FetchError extends Error { constructor(response) { - super(response.statusText) + super(`${response.status} ${response.statusText}`) this.response = response } } diff --git a/packages/react-async/src/useAsync.spec.js b/packages/react-async/src/useAsync.spec.js index af14be26..77a4602f 100644 --- a/packages/react-async/src/useAsync.spec.js +++ b/packages/react-async/src/useAsync.spec.js @@ -252,7 +252,7 @@ describe("useFetch", () => { }) test("throws a FetchError for failed requests", async () => { - const errorResponse = { ok: false, statusText: "Bad Request", json } + const errorResponse = { ok: false, status: 400, statusText: "Bad Request", json } globalScope.fetch.mockResolvedValue(errorResponse) const onResolve = jest.fn() const onReject = jest.fn() @@ -263,7 +263,7 @@ describe("useFetch", () => { expect(onReject).toHaveBeenCalled() let [err] = onReject.mock.calls[0] expect(err).toBeInstanceOf(FetchError) - expect(err.message).toEqual("Bad Request") + expect(err.message).toEqual("400 Bad Request") expect(err.response).toBe(errorResponse) }) }) From dba0bd76e934d5c1db3217a2065106db4177f747 Mon Sep 17 00:00:00 2001 From: Jacob Lee Date: Thu, 19 Sep 2019 11:20:44 -0500 Subject: [PATCH 3/3] Define FetchError as a class, not just an interface. This is necessary for TypeScript code to be permitted to use FetchError as a value at runtime, e.g. to perform an `instanceof FetchError` check. --- packages/react-async/src/index.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-async/src/index.d.ts b/packages/react-async/src/index.d.ts index eee2d9ff..77d88524 100644 --- a/packages/react-async/src/index.d.ts +++ b/packages/react-async/src/index.d.ts @@ -234,7 +234,7 @@ type FetchRun = { run(): void } -export interface FetchError extends Error { +export class FetchError extends Error { response: Response }