From 1acd60435f96a546a06b9a9d4ee53a185f60e506 Mon Sep 17 00:00:00 2001 From: Alex Mikhalev Date: Sat, 30 Jun 2018 23:26:48 -0600 Subject: [PATCH] Better rpc error handling --- app/sprinklers/websocket.ts | 10 +++--- common/sprinklers/websocketData.ts | 38 +++++++++++++++----- server/websocket/index.ts | 58 ++++++++++++++---------------- 3 files changed, 60 insertions(+), 46 deletions(-) diff --git a/app/sprinklers/websocket.ts b/app/sprinklers/websocket.ts index 97fdcdc..41447a3 100644 --- a/app/sprinklers/websocket.ts +++ b/app/sprinklers/websocket.ts @@ -36,9 +36,7 @@ export class WSSprinklersDevice extends s.SprinklersDevice { } async subscribe() { - if (!this.api.socket) { - throw new Error("WebSocket not connected"); - } + await this.api.authenticate("eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzcHJpbmtsZXJzMyIsImF1ZCI6IjA4NDQ4N2Q1LWU1NzktNDQ5YS05MzI5LTU5NWJlNGJjMmJiYyIsIm5hbWUiOiJBbGV4IE1pa2hhbGV2IiwidHlwZSI6ImFjY2VzcyIsImV4cCI6MTUzMDQxNzU3MCwiaWF0IjoxNTMwNDE1NzcwfQ.fRGiN_X1j3Hwe8a5y68wXLx1DQPtTkQr9h6Uh848dFM"); const subscribeRequest: ws.IDeviceSubscribeRequest = { deviceId: this.id, }; @@ -47,7 +45,7 @@ export class WSSprinklersDevice extends s.SprinklersDevice { this.connectionState.serverToBroker = true; this.connectionState.clientToServer = true; } catch (err) { - if ((err as ws.Error).code === ErrorCode.NoPermission) { + if ((err as ws.IError).code === ErrorCode.NoPermission) { this.connectionState.hasPermission = false; } else { log.error({ err }); @@ -117,7 +115,7 @@ export class WebSocketApiClient implements s.ISprinklersApi { // args must all be JSON serializable async makeDeviceCall(deviceId: string, request: deviceRequests.Request): Promise { if (this.socket == null) { - const error: ws.Error = { + const error: ws.IError = { code: ErrorCode.ServerDisconnected, message: "the server is not connected", }; @@ -264,7 +262,7 @@ export class WebSocketApiClient implements s.ISprinklersApi { } update(schema.sprinklersDevice, device, data.data); }, - error: (data: ws.Error) => { + error: (data: ws.IError) => { log.warn({ err: data }, "server error"); }, }; diff --git a/common/sprinklers/websocketData.ts b/common/sprinklers/websocketData.ts index 48b644b..0a7c9e5 100644 --- a/common/sprinklers/websocketData.ts +++ b/common/sprinklers/websocketData.ts @@ -1,6 +1,7 @@ import * as rpc from "../jsonRpc/index"; import { Response as ResponseData } from "@common/sprinklers/deviceRequests"; +import { ErrorCode } from "@common/sprinklers/ErrorCode"; export interface IAuthenticateRequest { accessToken: string; @@ -55,22 +56,41 @@ export interface IDeviceUpdate { export interface IServerNotificationTypes { "brokerConnectionUpdate": IBrokerConnectionUpdate; "deviceUpdate": IDeviceUpdate; - "error": Error; + "error": IError; } export type ServerNotificationMethod = keyof IServerNotificationTypes; -export type Error = rpc.DefaultErrorType; -export type ErrorData = rpc.ErrorData; +export type IError = rpc.DefaultErrorType; +export type ErrorData = rpc.ErrorData; -export type ServerMessage = rpc.Message<{}, IServerResponseTypes, Error, IServerNotificationTypes>; +export class RpcError extends Error implements IError { + name = "RpcError"; + code: number; + data: any; + + constructor(message: string, code: number = ErrorCode.BadRequest, data: any = {}) { + super(message); + this.code = code; + if (data instanceof Error) { + this.data = data.toString(); + } + this.data = data; + } + + toJSON(): IError { + return { code: this.code, message: this.message, data: this.data }; + } +} + +export type ServerMessage = rpc.Message<{}, IServerResponseTypes, IError, IServerNotificationTypes>; export type ServerNotification = rpc.Notification; -export type ServerResponse = rpc.Response; +export type ServerResponse = rpc.Response; export type ServerResponseData = - rpc.ResponseData; -export type ServerResponseHandlers = rpc.ResponseHandlers; + rpc.ResponseData; +export type ServerResponseHandlers = rpc.ResponseHandlers; export type ServerNotificationHandlers = rpc.NotificationHandlers; export type ClientRequest = rpc.Request; -export type ClientMessage = rpc.Message; -export type ClientRequestHandlers = rpc.RequestHandlers; +export type ClientMessage = rpc.Message; +export type ClientRequestHandlers = rpc.RequestHandlers; diff --git a/server/websocket/index.ts b/server/websocket/index.ts index 1586ae2..dc40e15 100644 --- a/server/websocket/index.ts +++ b/server/websocket/index.ts @@ -48,31 +48,33 @@ export class WebSocketClient { this.api.removeClient(this); } + private checkAuthorization() { + if (!this.userId) { + throw new ws.RpcError("this WebSocket session has not been authenticated", + ErrorCode.Unauthorized); + } + } + private requestHandlers: ws.ClientRequestHandlers = { authenticate: async (data: ws.IAuthenticateRequest) => { if (!data.accessToken) { - return { - result: "error", error: { - code: ErrorCode.BadRequest, message: "no token specified", - }, - }; + throw new ws.RpcError("no token specified", ErrorCode.BadRequest); } let decoded: TokenClaims; try { decoded = await verifyToken(data.accessToken); } catch (e) { - return { - result: "error", - error: { code: ErrorCode.BadToken, message: "invalid token", data: e }, - }; + throw new ws.RpcError("invalid token", ErrorCode.BadToken, e); } this.userId = decoded.aud; + log.info({ userId: decoded.aud, name: decoded.name }, "authenticated websocket client"); return { result: "success", data: { authenticated: true, message: "authenticated" }, }; }, deviceSubscribe: async (data: ws.IDeviceSubscribeRequest) => { + this.checkAuthorization(); const deviceId = data.deviceId; if (deviceId !== "grinklers") { // TODO: somehow validate this device id? return { @@ -100,6 +102,7 @@ export class WebSocketClient { return { result: "success", data: response }; }, deviceCall: async (data: ws.IDeviceCallRequest) => { + this.checkAuthorization(); try { const response = await this.doDeviceCallRequest(data); const resData: ws.IDeviceCallResponse = { @@ -108,13 +111,7 @@ export class WebSocketClient { return { result: "success", data: resData }; } catch (err) { const e: deviceRequests.ErrorResponseData = err; - return { - result: "error", error: { - code: e.code, - message: e.message, - data: e, - }, - }; + throw new ws.RpcError(e.message, e.code, e); } }, }; @@ -155,7 +152,6 @@ export class WebSocketClient { return this.onError({ socketData, err }, "received invalid websocket message from client", ErrorCode.Parse); } - log.debug({ data }, "client message"); switch (data.type) { case "request": await this.handleRequest(data); @@ -168,21 +164,21 @@ export class WebSocketClient { private async handleRequest(request: ws.ClientRequest) { let response: ws.ServerResponseData; - if (!this.requestHandlers[request.method]) { - log.warn({ method: request.method }, "received invalid client request method"); - response = { - result: "error", error: { - code: ErrorCode.BadRequest, message: "received invalid client request method", - }, - }; - } else { - try { - response = await rpc.handleRequest(this.requestHandlers, request); - } catch (err) { - log.error({ method: request.method, err }, "error during processing of client request"); + try { + if (!this.requestHandlers[request.method]) { + // noinspection ExceptionCaughtLocallyJS + throw new ws.RpcError("received invalid client request method"); + } + response = await rpc.handleRequest(this.requestHandlers, request); + } catch (err) { + if (err instanceof ws.RpcError) { + log.debug({ err }, "rpc error"); + response = { result: "error", error: err.toJSON() }; + } else { + log.error({ method: request.method, err }, "unhandled error during processing of client request"); response = { result: "error", error: { - code: ErrorCode.Internal, message: "error during processing of client request", + code: ErrorCode.Internal, message: "unhandled error during processing of client request", data: err.toString(), }, }; @@ -193,7 +189,7 @@ export class WebSocketClient { private onError(data: any, message: string, code: number = ErrorCode.Internal) { log.error(data, message); - const errorData: ws.Error = { code, message, data }; + const errorData: ws.IError = { code, message, data }; this.sendNotification("error", errorData); }