From 7ed3096b6fffe1cb3a5275e520a056292b431bc6 Mon Sep 17 00:00:00 2001 From: Alex Mikhalev Date: Sun, 8 Jul 2018 14:02:05 -0600 Subject: [PATCH] Improved error handling, and error display in login form --- app/pages/LoginPage.tsx | 13 ++++++++---- app/sprinklersRpc/websocketClient.ts | 2 +- app/state/HttpApi.ts | 21 +++++++++---------- app/state/TokenStore.ts | 6 +++--- .../express/errors.ts => common/ApiError.ts | 4 ++-- common/{sprinklersRpc => }/ErrorCode.ts | 0 common/{http.ts => httpApi/index.ts} | 0 common/sprinklersRpc/websocketData.ts | 2 +- server/express/authentication.ts | 7 +++---- server/express/errorHandler.ts | 14 +++++++++++++ server/express/index.ts | 3 +++ server/sprinklersRpc/websocketServer.ts | 2 +- 12 files changed, 47 insertions(+), 27 deletions(-) rename server/express/errors.ts => common/ApiError.ts (84%) rename common/{sprinklersRpc => }/ErrorCode.ts (100%) rename common/{http.ts => httpApi/index.ts} (100%) create mode 100644 server/express/errorHandler.ts diff --git a/app/pages/LoginPage.tsx b/app/pages/LoginPage.tsx index 9532f30..1ecdc7e 100644 --- a/app/pages/LoginPage.tsx +++ b/app/pages/LoginPage.tsx @@ -1,15 +1,17 @@ -import { AppState, injectState } from "@app/state"; -import log from "@common/logger"; import { computed, observable } from "mobx"; import { observer } from "mobx-react"; import * as React from "react"; -import { Container, Dimmer, Form, Header, InputOnChangeData, Loader, Segment } from "semantic-ui-react"; +import { Container, Dimmer, Form, Header, InputOnChangeData, Loader, Message, Segment } from "semantic-ui-react"; + +import { AppState, injectState } from "@app/state"; +import log from "@common/logger"; class LoginPageState { @observable username = ""; @observable password = ""; @observable loading: boolean = false; + @observable error: string | null = null; @computed get canLogin() { return this.username.length > 0 && this.password.length > 0; @@ -25,6 +27,7 @@ class LoginPageState { login(appState: AppState) { this.loading = true; + this.error = null; appState.tokenStore.grantPassword(this.username, this.password) .then(() => { this.loading = false; @@ -33,6 +36,7 @@ class LoginPageState { }) .catch((err) => { this.loading = false; + this.error = err.message; log.error({ err }, "login error"); }); } @@ -42,7 +46,7 @@ class LoginPage extends React.Component<{ appState: AppState }> { pageState = new LoginPageState(); render() { - const { username, password, canLogin, loading } = this.pageState; + const { username, password, canLogin, loading, error } = this.pageState; return ( @@ -59,6 +63,7 @@ class LoginPage extends React.Component<{ appState: AppState }> { type="password" onChange={this.pageState.onPasswordChange} /> + {error} Login diff --git a/app/sprinklersRpc/websocketClient.ts b/app/sprinklersRpc/websocketClient.ts index 45e846b..c6f5edf 100644 --- a/app/sprinklersRpc/websocketClient.ts +++ b/app/sprinklersRpc/websocketClient.ts @@ -2,10 +2,10 @@ import { action, observable, when } from "mobx"; import { update } from "serializr"; import { TokenStore } from "@app/state/TokenStore"; +import { ErrorCode } from "@common/ErrorCode"; import * as rpc from "@common/jsonRpc"; import logger from "@common/logger"; import * as deviceRequests from "@common/sprinklersRpc/deviceRequests"; -import { ErrorCode } from "@common/sprinklersRpc/ErrorCode"; import * as s from "@common/sprinklersRpc/index"; import * as schema from "@common/sprinklersRpc/schema/index"; import { seralizeRequest } from "@common/sprinklersRpc/schema/requests"; diff --git a/app/state/HttpApi.ts b/app/state/HttpApi.ts index 3774656..e80229a 100644 --- a/app/state/HttpApi.ts +++ b/app/state/HttpApi.ts @@ -1,14 +1,8 @@ import { TokenStore } from "@app/state/TokenStore"; +import ApiError from "@common/ApiError"; +import { ErrorCode } from "@common/ErrorCode"; -export class HttpApiError extends Error { - name = "HttpApiError"; - status: number; - - constructor(message: string, status: number = 500) { - super(message); - this.status = status; - } -} +export { ApiError }; export default class HttpApi { baseUrl: string; @@ -43,9 +37,14 @@ export default class HttpApi { ...options, }; const response = await fetch(this.baseUrl + url, options); - const responseBody = await response.json() || {}; + let responseBody: any; + try { + responseBody = await response.json() || {}; + } catch (e) { + throw new ApiError("Invalid JSON response", ErrorCode.Internal, e); + } if (!response.ok) { - throw new HttpApiError(responseBody.message || response.statusText, response.status); + throw new ApiError(responseBody.message || response.statusText, responseBody.code, responseBody.data); } return responseBody; } diff --git a/app/state/TokenStore.ts b/app/state/TokenStore.ts index 1432d9c..f336181 100644 --- a/app/state/TokenStore.ts +++ b/app/state/TokenStore.ts @@ -1,8 +1,8 @@ import { observable } from "mobx"; -import HttpApi, { HttpApiError } from "@app/state/HttpApi"; +import HttpApi, { ApiError } from "@app/state/HttpApi"; import { Token } from "@app/state/Token"; -import { TokenGrantPasswordRequest, TokenGrantRefreshRequest, TokenGrantResponse } from "@common/http"; +import { TokenGrantPasswordRequest, TokenGrantRefreshRequest, TokenGrantResponse } from "@common/httpApi"; import logger from "@common/logger"; const log = logger.child({ source: "TokenStore"}); @@ -52,7 +52,7 @@ export class TokenStore { async grantRefresh() { if (!this.refreshToken.isValid) { - throw new HttpApiError("can not grant refresh with invalid refresh_token"); + throw new ApiError("can not grant refresh with invalid refresh_token"); } const request: TokenGrantRefreshRequest = { grant_type: "refresh", refresh_token: this.refreshToken.token!, diff --git a/server/express/errors.ts b/common/ApiError.ts similarity index 84% rename from server/express/errors.ts rename to common/ApiError.ts index 9fd7b8e..2803420 100644 --- a/server/express/errors.ts +++ b/common/ApiError.ts @@ -1,6 +1,6 @@ -import { ErrorCode, toHttpStatus } from "@common/sprinklersRpc/ErrorCode"; +import { ErrorCode, toHttpStatus } from "@common/ErrorCode"; -export class ApiError extends Error { +export default class ApiError extends Error { name = "ApiError"; statusCode: number; code: ErrorCode; diff --git a/common/sprinklersRpc/ErrorCode.ts b/common/ErrorCode.ts similarity index 100% rename from common/sprinklersRpc/ErrorCode.ts rename to common/ErrorCode.ts diff --git a/common/http.ts b/common/httpApi/index.ts similarity index 100% rename from common/http.ts rename to common/httpApi/index.ts diff --git a/common/sprinklersRpc/websocketData.ts b/common/sprinklersRpc/websocketData.ts index 6ff33c1..d90f537 100644 --- a/common/sprinklersRpc/websocketData.ts +++ b/common/sprinklersRpc/websocketData.ts @@ -1,7 +1,7 @@ import * as rpc from "../jsonRpc/index"; +import { ErrorCode } from "@common/ErrorCode"; import { Response as ResponseData } from "@common/sprinklersRpc/deviceRequests"; -import { ErrorCode } from "@common/sprinklersRpc/ErrorCode"; export interface IAuthenticateRequest { accessToken: string; diff --git a/server/express/authentication.ts b/server/express/authentication.ts index 8e292b7..5690e7b 100644 --- a/server/express/authentication.ts +++ b/server/express/authentication.ts @@ -3,17 +3,16 @@ import Router from "express-promise-router"; import * as jwt from "jsonwebtoken"; import TokenClaims from "@common/TokenClaims"; - import { TokenGrantPasswordRequest, TokenGrantRefreshRequest, TokenGrantRequest, TokenGrantResponse, -} from "@common/http"; -import { ErrorCode } from "@common/sprinklersRpc/ErrorCode"; +} from "@common/httpApi"; +import { ErrorCode } from "@common/ErrorCode"; import { User } from "../models/User"; import { ServerState } from "../state"; -import { ApiError } from "./errors"; +import ApiError from "@common/ApiError"; export { TokenClaims }; diff --git a/server/express/errorHandler.ts b/server/express/errorHandler.ts new file mode 100644 index 0000000..c2de31b --- /dev/null +++ b/server/express/errorHandler.ts @@ -0,0 +1,14 @@ +import * as express from "express"; + +import ApiError from "@common/ApiError"; + +const errorHandler: express.ErrorRequestHandler = + (err: any, req: express.Request, res: express.Response, next: express.NextFunction) => { + if (err instanceof ApiError) { + res.status(err.statusCode).json(err.toJSON()); + } else { + next(err); + } + }; + +export default errorHandler; \ No newline at end of file diff --git a/server/express/index.ts b/server/express/index.ts index 76ffc9e..ced249a 100644 --- a/server/express/index.ts +++ b/server/express/index.ts @@ -9,6 +9,7 @@ import serveApp from "./serveApp"; import { User } from "../models/User"; import { authentication } from "./authentication"; +import errorHandler from "./errorHandler"; export function createApp(state: ServerState) { const app = express(); @@ -46,5 +47,7 @@ export function createApp(state: ServerState) { serveApp(app); + app.use(errorHandler); + return app; } diff --git a/server/sprinklersRpc/websocketServer.ts b/server/sprinklersRpc/websocketServer.ts index dac7ee9..49b6a40 100644 --- a/server/sprinklersRpc/websocketServer.ts +++ b/server/sprinklersRpc/websocketServer.ts @@ -5,7 +5,7 @@ import * as WebSocket from "ws"; import * as rpc from "@common/jsonRpc"; import log from "@common/logger"; import * as deviceRequests from "@common/sprinklersRpc/deviceRequests"; -import { ErrorCode } from "@common/sprinklersRpc/ErrorCode"; +import { ErrorCode } from "@common/ErrorCode"; import * as schema from "@common/sprinklersRpc/schema"; import * as ws from "@common/sprinklersRpc/websocketData"; import { TokenClaims, verifyToken } from "../express/authentication";