From 0d75952dc74fe6c7b67c12745842d4ba3577af5e Mon Sep 17 00:00:00 2001 From: Nathaniel Tucker Date: Thu, 8 Sep 2022 17:58:03 -0500 Subject: [PATCH] enhance: Better handling of errors (#2168) --- .../src/pages/IssueDetail/CommentForm.tsx | 6 +-- .../src/pages/IssueDetail/CommentInline.tsx | 1 - .../src/pages/IssueDetail/CreateComment.tsx | 2 +- .../src/pages/IssueDetail/CreateReaction.tsx | 13 +++--- .../github-app/src/resources/Reaction.tsx | 2 +- .../github-app/src/resources/Repository.tsx | 2 - packages/graphql/src/GQLEndpoint.ts | 23 +++++++--- packages/graphql/src/__tests__/gql.ts | 42 ++++++++++++++++++- 8 files changed, 69 insertions(+), 22 deletions(-) diff --git a/examples/github-app/src/pages/IssueDetail/CommentForm.tsx b/examples/github-app/src/pages/IssueDetail/CommentForm.tsx index 0ba734c7551..e92332041b0 100644 --- a/examples/github-app/src/pages/IssueDetail/CommentForm.tsx +++ b/examples/github-app/src/pages/IssueDetail/CommentForm.tsx @@ -1,9 +1,7 @@ -import { Card, Avatar, Form, Button, Input, Space } from 'antd'; -import { Link } from '@anansi/router'; +import { Form, Button, Input, Space } from 'antd'; import { useLoading } from '@rest-hooks/hooks'; -import { memo, useCallback } from 'react'; +import { memo } from 'react'; -const { Meta } = Card; const { TextArea } = Input; function CommentForm({ diff --git a/examples/github-app/src/pages/IssueDetail/CommentInline.tsx b/examples/github-app/src/pages/IssueDetail/CommentInline.tsx index fb522e03c32..41cacfa1f78 100644 --- a/examples/github-app/src/pages/IssueDetail/CommentInline.tsx +++ b/examples/github-app/src/pages/IssueDetail/CommentInline.tsx @@ -9,7 +9,6 @@ import rehypeHighlight from 'rehype-highlight'; import { UserResource } from 'resources/User'; import { css } from '@linaria/core'; import { EllipsisOutlined } from '@ant-design/icons'; -import { styled } from '@linaria/react'; import { CommentResource, Comment } from 'resources/Comment'; import FlexRow from 'components/FlexRow'; diff --git a/examples/github-app/src/pages/IssueDetail/CreateComment.tsx b/examples/github-app/src/pages/IssueDetail/CreateComment.tsx index e9057677b45..292a80be3a0 100644 --- a/examples/github-app/src/pages/IssueDetail/CreateComment.tsx +++ b/examples/github-app/src/pages/IssueDetail/CreateComment.tsx @@ -1,6 +1,6 @@ import UserResource from 'resources/User'; import { useCache, useController } from 'rest-hooks'; -import { Card, Avatar, Input } from 'antd'; +import { Card, Avatar } from 'antd'; import { Link } from '@anansi/router'; import { memo, useCallback } from 'react'; import { CommentResource } from 'resources/Comment'; diff --git a/examples/github-app/src/pages/IssueDetail/CreateReaction.tsx b/examples/github-app/src/pages/IssueDetail/CreateReaction.tsx index 2f499344e6a..ee1490771b2 100644 --- a/examples/github-app/src/pages/IssueDetail/CreateReaction.tsx +++ b/examples/github-app/src/pages/IssueDetail/CreateReaction.tsx @@ -1,5 +1,4 @@ -import { useSuspense, useController } from 'rest-hooks'; -import React from 'react'; +import { useController, useCache } from 'rest-hooks'; import { UserResource } from 'resources/User'; import { v4 as uuid } from 'uuid'; import { Reaction, ReactionResource, contentToIcon } from 'resources/Reaction'; @@ -18,13 +17,13 @@ export function CreateReaction({ issue: Issue; }) { const { fetch } = useController(); - const currentUser = useSuspense(UserResource.current); - const userReaction: Reaction | undefined = reactions.find( - (reaction) => reaction.user.login === currentUser.login, - ); + const currentUser = useCache(UserResource.current); + const userReaction: Reaction | undefined = + currentUser && + reactions.find((reaction) => reaction.user.login === currentUser.login); const handleClick = () => { - if (userReaction) return; + if (userReaction || !currentUser) return; fetch( ReactionResource.create, { diff --git a/examples/github-app/src/resources/Reaction.tsx b/examples/github-app/src/resources/Reaction.tsx index 37f7fd21c0f..227679710d3 100644 --- a/examples/github-app/src/resources/Reaction.tsx +++ b/examples/github-app/src/resources/Reaction.tsx @@ -70,7 +70,7 @@ export const ReactionResource = { ...rest, }), }), - getOptimisticResponse: (snap, params, body) => body, + getOptimisticResponse: (snap, params, body) => body as any, }), delete: base.delete.extend({ getOptimisticResponse: (snap, params) => params, diff --git a/examples/github-app/src/resources/Repository.tsx b/examples/github-app/src/resources/Repository.tsx index c251bd42180..a2ec0592edf 100644 --- a/examples/github-app/src/resources/Repository.tsx +++ b/examples/github-app/src/resources/Repository.tsx @@ -1,5 +1,3 @@ -import { RestGenerics } from '@rest-hooks/experimental'; - import { GithubEntity, createGithubResource, GithubGqlEndpoint } from './Base'; export class Repository extends GithubEntity { diff --git a/packages/graphql/src/GQLEndpoint.ts b/packages/graphql/src/GQLEndpoint.ts index 3b2ee5a5b57..5ed2ef54239 100644 --- a/packages/graphql/src/GQLEndpoint.ts +++ b/packages/graphql/src/GQLEndpoint.ts @@ -28,11 +28,20 @@ export default class GQLEndpoint< signal: this.signal ?? null, headers: this.getHeaders({ 'Content-Type': 'application/json' }), }), - ).then(async res => { - const { data, errors } = await res.json(); - if (errors) throw new GQLNetworkError(errors); - return data; - }); + ) + .then(async res => { + const json = await res.json(); + if (json.errors) throw new GQLNetworkError(json.errors); + if (!res.ok) throw new GQLNetworkError([json]); + return json.data; + }) + .catch(error => { + // ensure CORS, network down, and parse errors are still caught by NetworkErrorBoundary + if (error instanceof TypeError) { + (error as any).status = 400; + } + throw error; + }); }, args); return this; } @@ -54,6 +63,10 @@ export default class GQLEndpoint< return headers; } + errorPolicy(error: any) { + return error.status >= 500 ? ('soft' as const) : undefined; + } + query< Q extends string | ((variables: any) => string), S extends Schema | undefined, diff --git a/packages/graphql/src/__tests__/gql.ts b/packages/graphql/src/__tests__/gql.ts index dcc30543af2..2bc9bb455ea 100644 --- a/packages/graphql/src/__tests__/gql.ts +++ b/packages/graphql/src/__tests__/gql.ts @@ -61,7 +61,7 @@ describe('GQLEndpoint', () => { }, { 'content-type': 'application/json' }, ]; - else + else if (body.variables.name === 'Fong2') return [ 400, { @@ -80,6 +80,15 @@ describe('GQLEndpoint', () => { }, { 'content-type': 'application/json' }, ]; + else + return [ + 401, + { + message: 'This endpoint requires you to be authenticated.', + documentation_url: + 'https://docs.github.com/graphql/guides/forming-calls-with-graphql#authenticating-with-graphql', + }, + ]; }); }); afterAll(() => { @@ -161,4 +170,35 @@ describe('GQLEndpoint', () => { `[NetworkError: Generic failure]`, ); }); + + it('should throw if network response is not OK', async () => { + await expect(userDetail({ name: 'Fong3' })).rejects.toMatchInlineSnapshot( + `[NetworkError: This endpoint requires you to be authenticated.]`, + ); + }); + + it('should throw if network is down', async () => { + const oldError = console.error; + console.error = () => {}; + + nock('https://nosy-baritone.glitch.me') + .defaultReplyHeaders({ + 'Access-Control-Allow-Origin': '*', + 'Content-Type': 'application/json', + }) + .post('/') + .replyWithError(new TypeError('Network Down')); + + let error: any; + try { + await userDetail({ name: 'Fong4' }); + } catch (e) { + error = e; + } + expect(error).toBeDefined(); + expect(error.status).toBe(400); + + // eslint-disable-next-line require-atomic-updates + console.error = oldError; + }); });