From 60f5b7e3c0d744d070d2dea049496297a6e8c0df Mon Sep 17 00:00:00 2001 From: Kiwi Date: Fri, 1 Mar 2019 21:37:02 +0100 Subject: [PATCH] [CORL-282] Handle server errors in client (#2196) * feat: Handle server errors in client * refactor: use enum ERROR_TYPES * chore: better comment * fix: lint * fix: also look in queries for custom errors --- .../containers/ConfigureContainer.tsx | 6 +- .../admin/test/configure/general.spec.tsx | 41 +- .../framework/lib/errors/badUserInputError.ts | 78 --- src/core/client/framework/lib/errors/index.ts | 2 +- .../lib/errors/invalidRequestError.ts | 54 ++ .../lib/network/customErrorMiddleware.ts | 20 +- .../framework/lib/network/extractError.ts | 19 + .../client/framework/lib/network/index.ts | 1 + .../testHelpers/createRelayEnvironment.ts | 35 +- .../comments/components/EditCommentForm.tsx | 22 +- .../comments/components/PostCommentForm.tsx | 22 +- .../comments/components/ReplyCommentForm.tsx | 61 ++- .../containers/EditCommentFormContainer.tsx | 6 +- .../containers/PostCommentFormContainer.tsx | 6 +- .../containers/ReplyCommentFormContainer.tsx | 6 +- .../components/ReportCommentForm.tsx | 12 +- .../containers/ReportCommentFormContainer.tsx | 6 +- .../__snapshots__/editComment.spec.tsx.snap | 178 +++++- .../postLocalReply.spec.tsx.snap | 192 +++---- .../__snapshots__/postReply.spec.tsx.snap | 510 ++++++++++++++---- .../stream/test/comments/editComment.spec.tsx | 95 +++- .../stream/test/comments/postComment.spec.tsx | 102 ++-- .../stream/test/comments/postReply.spec.tsx | 109 ++-- .../test/comments/reportComment.spec.tsx | 50 +- src/core/client/stream/test/create.tsx | 2 + .../client/stream/test/createEnvironment.ts | 2 + src/core/common/errors.ts | 14 + src/core/server/errors/index.ts | 33 +- src/core/server/errors/translations.ts | 2 + .../server/graph/tenant/mutators/Comment.ts | 24 +- src/core/server/locales/en-US/errors.ftl | 3 + .../comments/pipeline/phases/commentLength.ts | 16 +- 32 files changed, 1256 insertions(+), 473 deletions(-) delete mode 100644 src/core/client/framework/lib/errors/badUserInputError.ts create mode 100644 src/core/client/framework/lib/errors/invalidRequestError.ts create mode 100644 src/core/client/framework/lib/network/extractError.ts diff --git a/src/core/client/admin/routes/configure/containers/ConfigureContainer.tsx b/src/core/client/admin/routes/configure/containers/ConfigureContainer.tsx index 5f4d35341..2d1490e5c 100644 --- a/src/core/client/admin/routes/configure/containers/ConfigureContainer.tsx +++ b/src/core/client/admin/routes/configure/containers/ConfigureContainer.tsx @@ -8,7 +8,7 @@ import { withUpdateSettingsMutation, } from "talk-admin/mutations"; import { TalkContext, withContext } from "talk-framework/lib/bootstrap"; -import { BadUserInputError } from "talk-framework/lib/errors"; +import { InvalidRequestError } from "talk-framework/lib/errors"; import { getMessage } from "talk-framework/lib/i18n"; import Configure from "../components/Configure"; @@ -86,8 +86,8 @@ class ConfigureContainer extends React.Component { } form.initialize(data); } catch (error) { - if (error instanceof BadUserInputError) { - return error.invalidArgsLocalized; + if (error instanceof InvalidRequestError) { + return error.invalidArgs; } // tslint:disable-next-line:no-console console.error(error); diff --git a/src/core/client/admin/test/configure/general.spec.tsx b/src/core/client/admin/test/configure/general.spec.tsx index f943651d2..3ee006546 100644 --- a/src/core/client/admin/test/configure/general.spec.tsx +++ b/src/core/client/admin/test/configure/general.spec.tsx @@ -2,6 +2,8 @@ import mockConsole from "jest-mock-console"; import { cloneDeep, get, merge } from "lodash"; import sinon from "sinon"; +import { ERROR_CODES } from "talk-common/errors"; +import { InvalidRequestError } from "talk-framework/lib/errors"; import { createSinonStub, replaceHistoryLocation, @@ -27,7 +29,10 @@ afterEach(() => { expect(console.error).not.toHaveBeenCalled(); }); -const createTestRenderer = async (resolver: any = {}) => { +const createTestRenderer = async ( + resolver: any = {}, + options: { muteNetworkErrors?: boolean } = {} +) => { const resolvers = { ...resolver, Query: { @@ -41,6 +46,7 @@ const createTestRenderer = async (resolver: any = {}) => { const { testRenderer } = create({ // Set this to true, to see graphql responses. logNetwork: false, + muteNetworkErrors: options.muteNetworkErrors, resolvers, initLocalState: localRecord => { localRecord.setValue(true, "loggedIn"); @@ -465,3 +471,36 @@ it("change closing comment streams", async () => { }); expect(updateSettingsStub.called).toBe(true); }); + +it("handle server error", async () => { + const updateSettingsStub = createSinonStub(s => + s.onFirstCall().callsFake((_: any, data: any) => { + throw new InvalidRequestError({ code: ERROR_CODES.INTERNAL_ERROR }); + }) + ); + const { configureContainer, generalContainer } = await createTestRenderer( + { + Mutation: { + updateSettings: updateSettingsStub, + }, + }, + { muteNetworkErrors: true } + ); + + const contentField = within(generalContainer).getByLabelText( + "Closed Stream Message" + ); + + // Let's change the content. + contentField.props.onChange("The stream has been closed"); + + // Send form + within(configureContainer) + .getByType("form") + .props.onSubmit(); + + // Look for internal error being displayed. + await waitForElement(() => + within(configureContainer).getByText("INTERNAL_ERROR") + ); +}); diff --git a/src/core/client/framework/lib/errors/badUserInputError.ts b/src/core/client/framework/lib/errors/badUserInputError.ts deleted file mode 100644 index e1674ae06..000000000 --- a/src/core/client/framework/lib/errors/badUserInputError.ts +++ /dev/null @@ -1,78 +0,0 @@ -import { mapValues, once } from "lodash"; -import { ReactNode } from "react"; -import { VALIDATION_REQUIRED } from "../messages"; - -/** - * ValidationError represents all possible string values - * that is responded by the server. - */ -type ValidationError = "REQUIRED"; - -/** - * InvalidArgsMap as responded by the server. - */ -interface InvalidArgsMap { - [key: string]: ValidationError; -} - -/** - * The localized version of `InvalidArgsMap`. - */ -interface InvalidArgsMapLocalilzed { - [key: string]: ReactNode; -} - -/** - * Shape of the `BadUserInput` extension. - */ -interface BadUserInputExtension { - code: "BAD_USER_INPUT"; - exception: { - invalidArgs: InvalidArgsMap; - }; -} - -/** - * Map server `ValidationError` to a translation message. - */ -const validationMap = { - REQUIRED: VALIDATION_REQUIRED, -}; - -/** - * BadUserInputError wraps the `BAD_USER_INPUT` error returned from the - * server. - */ -export default class BadUserInputError extends Error { - // Keep origin of original server response. - public readonly origin: BadUserInputExtension; - - constructor(error: BadUserInputExtension) { - super("BadUserInputError"); - - // Maintains proper stack trace for where our error was thrown. - if (Error.captureStackTrace) { - Error.captureStackTrace(this, BadUserInputError); - } - - this.origin = error; - } - - get invalidArgs(): InvalidArgsMap { - return this.origin.exception.invalidArgs; - } - - get invalidArgsLocalized(): InvalidArgsMapLocalilzed { - return this.computeInvalidArgsLocalized(); - } - - // Perform localization and memoize result. - private computeInvalidArgsLocalized = once(() => { - return mapValues(this.invalidArgs, v => { - if (v in validationMap) { - return validationMap[v](); - } - return v; - }); - }); -} diff --git a/src/core/client/framework/lib/errors/index.ts b/src/core/client/framework/lib/errors/index.ts index c1cbae255..fdbc76efd 100644 --- a/src/core/client/framework/lib/errors/index.ts +++ b/src/core/client/framework/lib/errors/index.ts @@ -1,2 +1,2 @@ export { default as UnknownServerError } from "./unknownServerError"; -export { default as BadUserInputError } from "./badUserInputError"; +export { default as InvalidRequestError } from "./invalidRequestError"; diff --git a/src/core/client/framework/lib/errors/invalidRequestError.ts b/src/core/client/framework/lib/errors/invalidRequestError.ts new file mode 100644 index 000000000..fe0655af7 --- /dev/null +++ b/src/core/client/framework/lib/errors/invalidRequestError.ts @@ -0,0 +1,54 @@ +import { FORM_ERROR } from "final-form"; +import { ERROR_CODES } from "talk-common/errors"; + +/** + * Shape of the `InvalidRequest` extension as + * the client requires. Note: the only crucial + * field is the `code` field. + */ +interface InvalidRequestExtension { + code: ERROR_CODES; + message?: string; + id?: string; + param?: string; +} + +/** + * InvalidRequestError wraps the `BAD_USER_INPUT` error returned from the + * server. + */ +export default class InvalidRequestError extends Error + implements InvalidRequestExtension { + // Keep extension of original server response. + public readonly extension: InvalidRequestExtension; + public readonly code: ERROR_CODES; + public readonly id?: string; + public readonly param?: string; + public readonly message: string; + public readonly extensions: string; + + constructor(extension: InvalidRequestExtension) { + super("InvalidRequestError"); + + // Maintains proper stack trace for where our error was thrown. + if (Error.captureStackTrace) { + Error.captureStackTrace(this, InvalidRequestError); + } + this.extension = extension; + this.code = extension.code; + this.id = extension.id; + this.param = extension.param; + this.message = extension.message || extension.code; + } + + get invalidArgs() { + if (this.param) { + return { + [this.param.substr("input.".length)]: this.message, + }; + } + return { + [FORM_ERROR]: this.message, + }; + } +} diff --git a/src/core/client/framework/lib/network/customErrorMiddleware.ts b/src/core/client/framework/lib/network/customErrorMiddleware.ts index 2efaf87d2..f0caf9623 100644 --- a/src/core/client/framework/lib/network/customErrorMiddleware.ts +++ b/src/core/client/framework/lib/network/customErrorMiddleware.ts @@ -1,26 +1,12 @@ import { Middleware } from "react-relay-network-modern/es"; -import { BadUserInputError, UnknownServerError } from "../errors"; -function getError(errors: Error[]): Error | null { - if (errors.length > 1 || !(errors[0] as any).extensions) { - // Multiple errors are GraphQL errors. - // TODO: (cvle) Is this assumption correct? - // No extensions == GraphQL error. - // TODO: (cvle) harmonize with server. - return null; - } - const err = errors[0]; - if ((err as any).code === "BAD_USER_INPUT") { - return new BadUserInputError((err as any).extensions); - } - return new UnknownServerError(err.message, (err as any).extensions); -} +import extractError from "./extractError"; const customErrorMiddleware: Middleware = next => async req => { const res = await next(req); - if (req.isMutation() && res.errors) { + if (res.errors) { // Extract custom error. - const error = getError(res.errors); + const error = extractError(res.errors); if (error) { throw error; } diff --git a/src/core/client/framework/lib/network/extractError.ts b/src/core/client/framework/lib/network/extractError.ts new file mode 100644 index 000000000..5430a5107 --- /dev/null +++ b/src/core/client/framework/lib/network/extractError.ts @@ -0,0 +1,19 @@ +import { ERROR_TYPES } from "talk-common/errors"; + +import { InvalidRequestError, UnknownServerError } from "../errors"; + +export default function extractError(errors: Error[]): Error | null { + if (errors.length > 1 || !(errors[0] as any).extensions) { + // Multiple errors are GraphQL errors. + // TODO: (cvle) Is this assumption correct? + // No extensions == GraphQL error. + // TODO: (cvle) harmonize with server. + return null; + } + // Handle custom errors here. + const err = errors[0]; + if ((err as any).extensions.type === ERROR_TYPES.INVALID_REQUEST_ERROR) { + return new InvalidRequestError((err as any).extensions); + } + return new UnknownServerError(err.message, (err as any).extensions); +} diff --git a/src/core/client/framework/lib/network/index.ts b/src/core/client/framework/lib/network/index.ts index 91065bfbd..42496d83d 100644 --- a/src/core/client/framework/lib/network/index.ts +++ b/src/core/client/framework/lib/network/index.ts @@ -1 +1,2 @@ export { default as createNetwork, TokenGetter } from "./createNetwork"; +export { default as extractError } from "./extractError"; diff --git a/src/core/client/framework/testHelpers/createRelayEnvironment.ts b/src/core/client/framework/testHelpers/createRelayEnvironment.ts index 17b00841c..a0edc2b11 100644 --- a/src/core/client/framework/testHelpers/createRelayEnvironment.ts +++ b/src/core/client/framework/testHelpers/createRelayEnvironment.ts @@ -1,5 +1,6 @@ +import { graphql, GraphQLSchema } from "graphql"; import { IResolvers } from "graphql-tools"; -import { createFetch } from "relay-local-schema"; + import { commitLocalUpdate, Environment, @@ -18,6 +19,7 @@ import { } from "talk-framework/lib/relay"; import { loadSchema } from "talk-common/graphql"; +import { InvalidRequestError } from "talk-framework/lib/errors"; export interface CreateRelayEnvironmentNetworkParams { /** project name of graphql-config */ @@ -50,6 +52,37 @@ export interface CreateRelayEnvironmentParams { source?: RecordSource; } +function createFetch({ + schema, + rootValue, + contextValue, +}: { + schema: GraphQLSchema; + rootValue?: any; + contextValue?: any; +}) { + return function fetchQuery(operation: any, variables: Record) { + return graphql( + schema, + operation.text, + rootValue, + contextValue, + variables + ).then(payload => { + if (payload.errors) { + payload.errors.forEach(e => { + // Throw our custom errors directly. + if (e.originalError instanceof InvalidRequestError) { + throw e.originalError; + } + }); + throw new Error(payload.errors.toString()); + } + return payload; + }); + }; +} + /** * create Relay environment for tests environments. */ diff --git a/src/core/client/stream/tabs/comments/components/EditCommentForm.tsx b/src/core/client/stream/tabs/comments/components/EditCommentForm.tsx index 716ace121..245ccfba2 100644 --- a/src/core/client/stream/tabs/comments/components/EditCommentForm.tsx +++ b/src/core/client/stream/tabs/comments/components/EditCommentForm.tsx @@ -20,7 +20,6 @@ import { Message, MessageIcon, RelativeTime, - Typography, ValidationMessage, } from "talk-ui/components"; @@ -51,7 +50,13 @@ const EditCommentForm: StatelessComponent = props => { const inputID = `comments-editCommentForm-rte-${props.id}`; return (
- {({ handleSubmit, submitting, hasValidationErrors, pristine }) => ( + {({ + handleSubmit, + submitting, + hasValidationErrors, + pristine, + submitError, + }) => ( = props => { {({ input, meta }) => ( -
+ Edit comment @@ -90,11 +95,16 @@ const EditCommentForm: StatelessComponent = props => { {meta.touched && (meta.error || meta.submitError) && ( - + {meta.error || meta.submitError} - + )} -
+ {submitError && ( + + {submitError} + + )} + )} {props.expired ? ( diff --git a/src/core/client/stream/tabs/comments/components/PostCommentForm.tsx b/src/core/client/stream/tabs/comments/components/PostCommentForm.tsx index 0de3bbbf3..7dbd34e50 100644 --- a/src/core/client/stream/tabs/comments/components/PostCommentForm.tsx +++ b/src/core/client/stream/tabs/comments/components/PostCommentForm.tsx @@ -5,17 +5,12 @@ import { Field, Form, FormSpy } from "react-final-form"; import { OnSubmit } from "talk-framework/lib/form"; import { required } from "talk-framework/lib/validation"; -import { - AriaInfo, - Button, - Flex, - HorizontalGutter, - Typography, -} from "talk-ui/components"; +import { AriaInfo, Button, Flex, HorizontalGutter } from "talk-ui/components"; import PoweredBy from "./PoweredBy"; import RTE from "./RTE"; +import ValidationMessage from "talk-admin/routes/configure/components/ValidationMessage"; import styles from "./PostCommentForm.css"; interface FormProps { @@ -30,7 +25,7 @@ export interface PostCommentFormProps { const PostCommentForm: StatelessComponent = props => ( - {({ handleSubmit, submitting, hasValidationErrors }) => ( + {({ handleSubmit, submitting, hasValidationErrors, submitError }) => ( = props => ( {({ input, meta }) => ( -
+ = props => ( {meta.touched && (meta.error || meta.submitError) && ( - + {meta.error || meta.submitError} - + )} -
+ {submitError && ( + {submitError} + )} + )} = props => { const inputID = `comments-replyCommentForm-rte-${props.id}`; return ( - {({ handleSubmit, submitting, hasValidationErrors }) => ( + {({ handleSubmit, submitting, hasValidationErrors, submitError }) => ( = props => { {({ input, meta }) => ( -
- - - Write a reply - - - {props.parentUsername && ( - - )} - - input.onChange(html)} - value={input.value} - placeholder="Write a reply" - forwardRef={props.rteRef} - disabled={submitting} - /> - + +
+ + + Write a reply + + + {props.parentUsername && ( + + )} + + input.onChange(html)} + value={input.value} + placeholder="Write a reply" + forwardRef={props.rteRef} + disabled={submitting} + /> + +
{meta.touched && (meta.error || meta.submitError) && ( - + {meta.error || meta.submitError} - + )} -
+ {submitError && ( + + {submitError} + + )} + )} diff --git a/src/core/client/stream/tabs/comments/containers/EditCommentFormContainer.tsx b/src/core/client/stream/tabs/comments/containers/EditCommentFormContainer.tsx index 4b291ffbe..ec53b4df9 100644 --- a/src/core/client/stream/tabs/comments/containers/EditCommentFormContainer.tsx +++ b/src/core/client/stream/tabs/comments/containers/EditCommentFormContainer.tsx @@ -4,7 +4,7 @@ import { graphql } from "react-relay"; import { isBeforeDate } from "talk-common/utils"; import { withContext } from "talk-framework/lib/bootstrap"; -import { BadUserInputError } from "talk-framework/lib/errors"; +import { InvalidRequestError } from "talk-framework/lib/errors"; import { withFragmentContainer } from "talk-framework/lib/relay"; import { PropTypesOf } from "talk-framework/types"; @@ -82,8 +82,8 @@ export class EditCommentFormContainer extends Component { this.props.onClose(); } } catch (error) { - if (error instanceof BadUserInputError) { - return error.invalidArgsLocalized; + if (error instanceof InvalidRequestError) { + return error.invalidArgs; } // tslint:disable-next-line:no-console console.error(error); diff --git a/src/core/client/stream/tabs/comments/containers/PostCommentFormContainer.tsx b/src/core/client/stream/tabs/comments/containers/PostCommentFormContainer.tsx index 25791bd9b..065f75300 100644 --- a/src/core/client/stream/tabs/comments/containers/PostCommentFormContainer.tsx +++ b/src/core/client/stream/tabs/comments/containers/PostCommentFormContainer.tsx @@ -1,7 +1,7 @@ import React, { Component } from "react"; import { withContext } from "talk-framework/lib/bootstrap"; -import { BadUserInputError } from "talk-framework/lib/errors"; +import { InvalidRequestError } from "talk-framework/lib/errors"; import { PromisifiedStorage } from "talk-framework/lib/storage"; import { PropTypesOf } from "talk-framework/types"; @@ -59,8 +59,8 @@ export class PostCommentFormContainer extends Component { }); form.reset({}); } catch (error) { - if (error instanceof BadUserInputError) { - return error.invalidArgsLocalized; + if (error instanceof InvalidRequestError) { + return error.invalidArgs; } // tslint:disable-next-line:no-console console.error(error); diff --git a/src/core/client/stream/tabs/comments/containers/ReplyCommentFormContainer.tsx b/src/core/client/stream/tabs/comments/containers/ReplyCommentFormContainer.tsx index e4f28c615..d8ab4a213 100644 --- a/src/core/client/stream/tabs/comments/containers/ReplyCommentFormContainer.tsx +++ b/src/core/client/stream/tabs/comments/containers/ReplyCommentFormContainer.tsx @@ -3,7 +3,7 @@ import React, { Component } from "react"; import { graphql } from "react-relay"; import { withContext } from "talk-framework/lib/bootstrap"; -import { BadUserInputError } from "talk-framework/lib/errors"; +import { InvalidRequestError } from "talk-framework/lib/errors"; import { withFragmentContainer } from "talk-framework/lib/relay"; import { PromisifiedStorage } from "talk-framework/lib/storage"; import { PropTypesOf } from "talk-framework/types"; @@ -87,8 +87,8 @@ export class ReplyCommentFormContainer extends Component { this.props.onClose(); } } catch (error) { - if (error instanceof BadUserInputError) { - return error.invalidArgsLocalized; + if (error instanceof InvalidRequestError) { + return error.invalidArgs; } // tslint:disable-next-line:no-console console.error(error); diff --git a/src/core/client/stream/tabs/comments/views/reportComment/components/ReportCommentForm.tsx b/src/core/client/stream/tabs/comments/views/reportComment/components/ReportCommentForm.tsx index 1051f6443..a4d028313 100644 --- a/src/core/client/stream/tabs/comments/views/reportComment/components/ReportCommentForm.tsx +++ b/src/core/client/stream/tabs/comments/views/reportComment/components/ReportCommentForm.tsx @@ -12,6 +12,7 @@ import { HorizontalGutter, RadioButton, Typography, + ValidationMessage, } from "talk-ui/components"; import PropagateMount from "./PropagateMount"; @@ -62,7 +63,13 @@ class ReportCommentForm extends React.Component { const { onCancel, onSubmit, onResize, id } = this.props; return ( - {({ handleSubmit, submitting, hasValidationErrors, form }) => ( + {({ + handleSubmit, + submitting, + hasValidationErrors, + form, + submitError, + }) => ( { )} + {submitError && ( + {submitError} + )} {get(form.getFieldState("reason"), "value") && ( diff --git a/src/core/client/stream/tabs/comments/views/reportComment/containers/ReportCommentFormContainer.tsx b/src/core/client/stream/tabs/comments/views/reportComment/containers/ReportCommentFormContainer.tsx index c8326dfba..9448d697e 100644 --- a/src/core/client/stream/tabs/comments/views/reportComment/containers/ReportCommentFormContainer.tsx +++ b/src/core/client/stream/tabs/comments/views/reportComment/containers/ReportCommentFormContainer.tsx @@ -1,7 +1,7 @@ import React, { Component } from "react"; import { graphql } from "react-relay"; -import { BadUserInputError } from "talk-framework/lib/errors"; +import { InvalidRequestError } from "talk-framework/lib/errors"; import { withFragmentContainer } from "talk-framework/lib/relay"; import { PropTypesOf } from "talk-framework/types"; import { ReportCommentFormContainer_comment as CommentData } from "talk-stream/__generated__/ReportCommentFormContainer_comment.graphql"; @@ -52,8 +52,8 @@ export class ReportCommentFormContainer extends Component { } this.setState({ done: true }); } catch (error) { - if (error instanceof BadUserInputError) { - return error.invalidArgsLocalized; + if (error instanceof InvalidRequestError) { + return error.invalidArgs; } // tslint:disable-next-line:no-console console.error(error); diff --git a/src/core/client/stream/test/comments/__snapshots__/editComment.spec.tsx.snap b/src/core/client/stream/test/comments/__snapshots__/editComment.spec.tsx.snap index 897480ce2..b048b901f 100644 --- a/src/core/client/stream/test/comments/__snapshots__/editComment.spec.tsx.snap +++ b/src/core/client/stream/test/comments/__snapshots__/editComment.spec.tsx.snap @@ -180,6 +180,172 @@ exports[`cancel edit 1`] = ` `; +exports[`edit a comment and handle server error: edit form 1`] = ` +
+ +
+
+
+ + Markus + + +
+
+
+ +
+
+
+
+ + + +
+
+
+
+
+ + + Edit: + + remaining + +
+
+ + +
+
+ +
+`; + exports[`edit a comment: edit form 1`] = `
-
+
-
+
-
+