From 1a9c1f5cbc184e0c4ee7ae1d89ea556615826339 Mon Sep 17 00:00:00 2001 From: Alex Mikhalev Date: Mon, 25 Jun 2018 17:37:36 -0600 Subject: [PATCH] Improved error handling --- app/components/DeviceView.tsx | 3 +- app/components/RunSectionForm.tsx | 22 ++++++++++++-- app/sprinklers/websocket.ts | 17 ++++++++++- app/state/StateBase.ts | 2 +- app/state/{ui.ts => UiStore.ts} | 0 app/state/index.ts | 4 +-- app/state/inject.tsx | 44 --------------------------- app/state/reactContext.tsx | 50 +++++++++++++++++++++++++++++++ common/sprinklers/ErrorCode.ts | 9 ++++++ common/sprinklers/requests.ts | 7 +++-- server/websocket/index.ts | 4 +-- yarn.lock | 24 +++++++-------- 12 files changed, 118 insertions(+), 68 deletions(-) rename app/state/{ui.ts => UiStore.ts} (100%) delete mode 100644 app/state/inject.tsx create mode 100644 app/state/reactContext.tsx create mode 100644 common/sprinklers/ErrorCode.ts diff --git a/app/components/DeviceView.tsx b/app/components/DeviceView.tsx index 323eb12..6e5cc69 100644 --- a/app/components/DeviceView.tsx +++ b/app/components/DeviceView.tsx @@ -52,6 +52,7 @@ class DeviceView extends React.Component { render() { const { id, connectionState, sections, programs, sectionRunner } = this.device; + const { uiStore } = this.props.state; return ( @@ -68,7 +69,7 @@ class DeviceView extends React.Component { - + diff --git a/app/components/RunSectionForm.tsx b/app/components/RunSectionForm.tsx index 9b0d364..c3d732a 100644 --- a/app/components/RunSectionForm.tsx +++ b/app/components/RunSectionForm.tsx @@ -3,14 +3,17 @@ import { observer } from "mobx-react"; import * as React from "react"; import { DropdownItemProps, DropdownProps, Form, Header, Segment } from "semantic-ui-react"; +import { UiStore } from "@app/state"; import { Duration } from "@common/Duration"; import log from "@common/logger"; import { Section } from "@common/sprinklers"; +import { RunSectionResponse } from "@common/sprinklers/requests"; import DurationInput from "./DurationInput"; @observer export default class RunSectionForm extends React.Component<{ sections: Section[], + uiStore: UiStore, }, { duration: Duration, section: number | "", @@ -70,8 +73,23 @@ export default class RunSectionForm extends React.Component<{ const section: Section = this.props.sections[this.state.section]; const { duration } = this.state; section.run(duration.toSeconds()) - .then((result) => log.debug({ result }, "requested section run")) - .catch((err) => log.error(err, "error running section")); + .then(this.onRunSuccess) + .catch(this.onRunError); + } + + private onRunSuccess = (result: RunSectionResponse) => { + log.debug({ result }, "requested section run"); + this.props.uiStore.addMessage({ + color: "green", header: "Section running", + }); + } + + private onRunError = (err: RunSectionResponse) => { + log.error(err, "error running section"); + this.props.uiStore.addMessage({ + color: "red", header: "Error running section", + content: err.message, + }); } private get isValid(): boolean { diff --git a/app/sprinklers/websocket.ts b/app/sprinklers/websocket.ts index 13b95db..92c84b3 100644 --- a/app/sprinklers/websocket.ts +++ b/app/sprinklers/websocket.ts @@ -1,6 +1,7 @@ import { update } from "serializr"; import logger from "@common/logger"; +import { ErrorCode } from "@common/sprinklers/ErrorCode"; import * as s from "@common/sprinklers/index"; import * as requests from "@common/sprinklers/requests"; import * as schema from "@common/sprinklers/schema/index"; @@ -10,6 +11,8 @@ import { action, autorun, observable } from "mobx"; const log = logger.child({ source: "websocket" }); +const TIMEOUT_MS = 5000; + export class WSSprinklersDevice extends s.SprinklersDevice { readonly api: WebSocketApiClient; @@ -84,14 +87,26 @@ export class WebSocketApiClient implements s.ISprinklersApi { id, deviceName, data: requestData, }; const promise = new Promise((resolve, reject) => { + let timeoutHandle: number; this.deviceResponseCallbacks[id] = (resData) => { + clearTimeout(timeoutHandle); + delete this.deviceResponseCallbacks[id]; if (resData.data.result === "success") { resolve(resData.data); } else { reject(resData.data); } - delete this.deviceResponseCallbacks[id]; }; + timeoutHandle = setTimeout(() => { + delete this.deviceResponseCallbacks[id]; + const res: requests.RunSectionResponse = { + type: "runSection", + result: "error", + code: ErrorCode.Timeout, + message: "the request timed out", + }; + reject(res); + }, TIMEOUT_MS); }); this.socket.send(JSON.stringify(data)); return promise; diff --git a/app/state/StateBase.ts b/app/state/StateBase.ts index 0f0d2cb..5fdf578 100644 --- a/app/state/StateBase.ts +++ b/app/state/StateBase.ts @@ -1,5 +1,5 @@ import { ISprinklersApi } from "@common/sprinklers"; -import { UiStore } from "./ui"; +import { UiStore } from "./UiStore"; export default abstract class StateBase { abstract readonly sprinklersApi: ISprinklersApi; diff --git a/app/state/ui.ts b/app/state/UiStore.ts similarity index 100% rename from app/state/ui.ts rename to app/state/UiStore.ts diff --git a/app/state/index.ts b/app/state/index.ts index 613c3de..522e323 100644 --- a/app/state/index.ts +++ b/app/state/index.ts @@ -1,3 +1,3 @@ -export { UiMessage, UiStore } from "./ui"; -export * from "./inject"; +export { UiMessage, UiStore } from "./UiStore"; +export * from "./reactContext"; export { default as StateBase } from "./StateBase"; diff --git a/app/state/inject.tsx b/app/state/inject.tsx deleted file mode 100644 index ac1f650..0000000 --- a/app/state/inject.tsx +++ /dev/null @@ -1,44 +0,0 @@ -import * as PropTypes from "prop-types"; -import * as React from "react"; - -import { StateBase } from "@app/state"; - -interface IProvidedStateContext { - providedState: StateBase; -} - -const providedStateContextTypes: PropTypes.ValidationMap = { - providedState: PropTypes.object, -}; - -export class ProvideState extends React.Component<{ - state: StateBase, -}> implements React.ChildContextProvider { - static childContextTypes = providedStateContextTypes; - - getChildContext(): IProvidedStateContext { - return { - providedState: this.props.state, - }; - } - - render() { - return React.Children.only(this.props.children); - } -} - -type Diff = - ({[P in T]: P } & {[P in U]: never } & { [x: string]: never })[T]; -type Omit = {[P in Diff]: T[P]}; - -export function injectState

(Component: React.ComponentType

) { - return class extends React.Component> { - static contextTypes = providedStateContextTypes; - context!: IProvidedStateContext; - - render() { - const state = this.context.providedState; - return ; - } - }; -} diff --git a/app/state/reactContext.tsx b/app/state/reactContext.tsx new file mode 100644 index 0000000..387d7e7 --- /dev/null +++ b/app/state/reactContext.tsx @@ -0,0 +1,50 @@ +import * as React from "react"; + +import { StateBase } from "@app/state"; + +const StateContext = React.createContext(null); + +export interface ProvideStateProps { + state: StateBase; + children: React.ReactNode; +} + +export function ProvideState({state, children}: ProvideStateProps) { + return ( + + {children} + + ); +} + +export interface ConsumeStateProps { + children: (state: StateBase) => React.ReactNode; +} + +export function ConsumeState({children}: ConsumeStateProps) { + const consumeState = (state: StateBase | null) => { + if (state == null) { + throw new Error("Component with ConsumeState must be mounted inside ProvideState"); + } + return children(state); + }; + return {consumeState}; +} + +type Diff = + ({[P in T]: P } & {[P in U]: never } & { [x: string]: never })[T]; +type Omit = {[P in Diff]: T[P]}; + +export function injectState

(Component: React.ComponentType

) { + return class extends React.Component> { + render() { + const consumeState = (state: StateBase | null) => { + if (state == null) { + throw new Error("Component with injectState must be mounted inside ProvideState"); + } + return ; + }; + return {consumeState}; + } + }; +} diff --git a/common/sprinklers/ErrorCode.ts b/common/sprinklers/ErrorCode.ts new file mode 100644 index 0000000..7710328 --- /dev/null +++ b/common/sprinklers/ErrorCode.ts @@ -0,0 +1,9 @@ +export enum ErrorCode { + BadRequest = 100, + NotSpecified = 101, + Parse = 102, + Range = 103, + InvalidData = 104, + Internal = 200, + Timeout = 300, +} diff --git a/common/sprinklers/requests.ts b/common/sprinklers/requests.ts index c688e45..056a725 100644 --- a/common/sprinklers/requests.ts +++ b/common/sprinklers/requests.ts @@ -37,9 +37,10 @@ export interface SuccessResponseData extends WithT export interface ErrorResponseData extends WithType { result: "error"; - error: string; - offset?: number; - code?: number; + message: string; + code: number; + name?: string; + cause?: any; } export type Response = diff --git a/server/websocket/index.ts b/server/websocket/index.ts index fe1cb72..c4fdda7 100644 --- a/server/websocket/index.ts +++ b/server/websocket/index.ts @@ -33,11 +33,11 @@ export class WebSocketApi { const stop = () => { disposers.forEach((disposer) => disposer()); }; - socket.on("message", this.handleSocketMessage); + socket.on("message", (data) => this.handleSocketMessage(socket, data)); socket.on("close", () => stop()); } - private handleSocketMessage = (socket: WebSocket, socketData: WebSocket.Data) => { + private handleSocketMessage(socket: WebSocket, socketData: WebSocket.Data) { if (typeof socketData !== "string") { return log.error({ type: typeof socketData }, "received invalid socket data type from client"); } diff --git a/yarn.lock b/yarn.lock index 89d9997..b304dfe 100644 --- a/yarn.lock +++ b/yarn.lock @@ -168,7 +168,7 @@ "@webassemblyjs/helper-code-frame@1.5.12": version "1.5.12" - resolved "https://registry.yarnpkg.com/@webassemblyjs/helper-code-frame/-/helper-code-frame-1.5.12.tgz#3cdc1953093760d1c0f0caf745ccd62bdb6627c7" + resolved "https://registry.yarnpkg.com/@webassemblyjs/helper-errorCode-frame/-/helper-errorCode-frame-1.5.12.tgz#3cdc1953093760d1c0f0caf745ccd62bdb6627c7" dependencies: "@webassemblyjs/wast-printer" "1.5.12" @@ -265,7 +265,7 @@ "@webassemblyjs/ast" "1.5.12" "@webassemblyjs/floating-point-hex-parser" "1.5.12" "@webassemblyjs/helper-api-error" "1.5.12" - "@webassemblyjs/helper-code-frame" "1.5.12" + "@webassemblyjs/helper-errorCode-frame" "1.5.12" "@webassemblyjs/helper-fsm" "1.5.12" long "^3.2.0" mamacro "^0.0.3" @@ -564,7 +564,7 @@ aws4@^1.2.1: babel-code-frame@6.26.0, babel-code-frame@^6.22.0, babel-code-frame@^6.26.0: version "6.26.0" - resolved "https://registry.yarnpkg.com/babel-code-frame/-/babel-code-frame-6.26.0.tgz#63fd43f7dc1e3bb7ce35947db8fe369a3f58c74b" + resolved "https://registry.yarnpkg.com/babel-errorCode-frame/-/babel-errorCode-frame-6.26.0.tgz#63fd43f7dc1e3bb7ce35947db8fe369a3f58c74b" dependencies: chalk "^1.1.3" esutils "^2.0.2" @@ -1076,7 +1076,7 @@ coa@~1.0.1: code-point-at@^1.0.0: version "1.1.0" - resolved "https://registry.yarnpkg.com/code-point-at/-/code-point-at-1.1.0.tgz#0d070b4d043a5bea33a2f1a40e2edb3d9a4ccf77" + resolved "https://registry.yarnpkg.com/errorCode-point-at/-/errorCode-point-at-1.1.0.tgz#0d070b4d043a5bea33a2f1a40e2edb3d9a4ccf77" collection-visit@^1.0.0: version "1.0.0" @@ -1392,7 +1392,7 @@ css-loader@^0.28.11: version "0.28.11" resolved "https://registry.yarnpkg.com/css-loader/-/css-loader-0.28.11.tgz#c3f9864a700be2711bb5a2462b2389b1a392dab7" dependencies: - babel-code-frame "^6.26.0" + babel-errorCode-frame "^6.26.0" css-selector-tokenizer "^0.7.0" cssnano "^3.10.0" icss-utils "^2.1.0" @@ -2960,13 +2960,13 @@ is-finite@^1.0.0: is-fullwidth-code-point@^1.0.0: version "1.0.0" - resolved "https://registry.yarnpkg.com/is-fullwidth-code-point/-/is-fullwidth-code-point-1.0.0.tgz#ef9e31386f031a7f0d643af82fde50c457ef00cb" + resolved "https://registry.yarnpkg.com/is-fullwidth-errorCode-point/-/is-fullwidth-errorCode-point-1.0.0.tgz#ef9e31386f031a7f0d643af82fde50c457ef00cb" dependencies: number-is-nan "^1.0.0" is-fullwidth-code-point@^2.0.0: version "2.0.0" - resolved "https://registry.yarnpkg.com/is-fullwidth-code-point/-/is-fullwidth-code-point-2.0.0.tgz#a3b30a5c4f199183167aaab93beefae3ddfb654f" + resolved "https://registry.yarnpkg.com/is-fullwidth-errorCode-point/-/is-fullwidth-errorCode-point-2.0.0.tgz#a3b30a5c4f199183167aaab93beefae3ddfb654f" is-glob@^3.1.0: version "3.1.0" @@ -5083,7 +5083,7 @@ react-dev-utils@^5.0.1: resolved "https://registry.yarnpkg.com/react-dev-utils/-/react-dev-utils-5.0.1.tgz#1f396e161fe44b595db1b186a40067289bf06613" dependencies: address "1.0.3" - babel-code-frame "6.26.0" + babel-errorCode-frame "6.26.0" chalk "1.1.3" cross-spawn "5.1.0" detect-port-alt "1.1.6" @@ -5973,15 +5973,15 @@ string-width@^1.0.1, string-width@^1.0.2: version "1.0.2" resolved "https://registry.yarnpkg.com/string-width/-/string-width-1.0.2.tgz#118bdf5b8cdc51a2a7e70d211e07e2b0b9b107d3" dependencies: - code-point-at "^1.0.0" - is-fullwidth-code-point "^1.0.0" + errorCode-point-at "^1.0.0" + is-fullwidth-errorCode-point "^1.0.0" strip-ansi "^3.0.0" "string-width@^1.0.2 || 2", string-width@^2.0.0, string-width@^2.1.0, string-width@^2.1.1: version "2.1.1" resolved "https://registry.yarnpkg.com/string-width/-/string-width-2.1.1.tgz#ab93f27a8dc13d28cac815c462143a6d9012ae9e" dependencies: - is-fullwidth-code-point "^2.0.0" + is-fullwidth-errorCode-point "^2.0.0" strip-ansi "^4.0.0" string.prototype.padend@^3.0.0: @@ -6242,7 +6242,7 @@ tslint@^5.10.0: version "5.10.0" resolved "https://registry.yarnpkg.com/tslint/-/tslint-5.10.0.tgz#11e26bccb88afa02dd0d9956cae3d4540b5f54c3" dependencies: - babel-code-frame "^6.22.0" + babel-errorCode-frame "^6.22.0" builtin-modules "^1.1.1" chalk "^2.3.0" commander "^2.12.1"