From 28f9dde4c82c56791467b11b2302543f83d743b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Mon, 11 Dec 2017 10:14:16 +0100 Subject: [PATCH] refactor(rest): use named functions for sequence actions Refactor sequence-action providers to use a named "action" method to hold the actual action implementation, and change the anonymous lambda (arrow) functions returned by "value" methods to delegate to these new "action" methods. This improves error stack traces in cases where the action fails. Before this change, there would be no reference to the actual sequence action that failed. After this change, the stack trace contains an entry for "{Action}Provider.action" function. --- packages/rest/src/providers/bind-element.ts | 9 +++-- packages/rest/src/providers/find-route.ts | 14 +++++--- .../rest/src/providers/get-from-context.ts | 10 ++++-- packages/rest/src/providers/invoke-method.ts | 11 ++++--- .../rest/src/providers/log-error-provider.ts | 33 +++++++++++-------- packages/rest/src/providers/reject.ts | 18 +++++----- 6 files changed, 59 insertions(+), 36 deletions(-) diff --git a/packages/rest/src/providers/bind-element.ts b/packages/rest/src/providers/bind-element.ts index b2f5f73ac621..699c1847d133 100644 --- a/packages/rest/src/providers/bind-element.ts +++ b/packages/rest/src/providers/bind-element.ts @@ -3,13 +3,18 @@ // This file is licensed under the MIT License. // License text available at https://opensource.org/licenses/MIT -import {Context, inject, Provider} from '@loopback/context'; +import {Context, inject, Provider, Binding} from '@loopback/context'; import {BindElement} from '../internal-types'; import {RestBindings} from '../keys'; export class BindElementProvider implements Provider { constructor(@inject(RestBindings.Http.CONTEXT) protected context: Context) {} + value(): BindElement { - return (key: string) => this.context.bind(key); + return key => this.action(key); + } + + action(key: string): Binding { + return this.context.bind(key); } } diff --git a/packages/rest/src/providers/find-route.ts b/packages/rest/src/providers/find-route.ts index 4d7ef5a5d130..0bfc5cb65837 100644 --- a/packages/rest/src/providers/find-route.ts +++ b/packages/rest/src/providers/find-route.ts @@ -7,6 +7,8 @@ import {Context, inject, Provider} from '@loopback/context'; import {FindRoute} from '../internal-types'; import {HttpHandler} from '../http-handler'; import {RestBindings} from '../keys'; +import {ParsedRequest} from '../internal-types'; +import {ResolvedRoute} from '../router/routing-table'; export class FindRouteProvider implements Provider { constructor( @@ -15,10 +17,12 @@ export class FindRouteProvider implements Provider { ) {} value(): FindRoute { - return request => { - const found = this.handler.findRoute(request); - found.updateBindings(this.context); - return found; - }; + return request => this.action(request); + } + + action(request: ParsedRequest): ResolvedRoute { + const found = this.handler.findRoute(request); + found.updateBindings(this.context); + return found; } } diff --git a/packages/rest/src/providers/get-from-context.ts b/packages/rest/src/providers/get-from-context.ts index 190d3bb3a6ce..e3ed0957dd7c 100644 --- a/packages/rest/src/providers/get-from-context.ts +++ b/packages/rest/src/providers/get-from-context.ts @@ -3,14 +3,18 @@ // This file is licensed under the MIT License. // License text available at https://opensource.org/licenses/MIT -import {Context, inject, Provider} from '@loopback/context'; +import {Context, inject, Provider, BoundValue} from '@loopback/context'; import {GetFromContext} from '../internal-types'; import {RestBindings} from '../keys'; export class GetFromContextProvider implements Provider { constructor(@inject(RestBindings.Http.CONTEXT) protected context: Context) {} - value() { - return (key: string) => this.context.get(key); + value(): GetFromContext { + return key => this.action(key); + } + + action(key: string): Promise { + return this.context.get(key); } } diff --git a/packages/rest/src/providers/invoke-method.ts b/packages/rest/src/providers/invoke-method.ts index 7f2388c659bc..3b85944b6739 100644 --- a/packages/rest/src/providers/invoke-method.ts +++ b/packages/rest/src/providers/invoke-method.ts @@ -4,15 +4,18 @@ // License text available at https://opensource.org/licenses/MIT import {Context, inject, Provider} from '@loopback/context'; -import {InvokeMethod} from '../internal-types'; +import {InvokeMethod, OperationArgs, OperationRetval} from '../internal-types'; import {RestBindings} from '../keys'; +import {RouteEntry} from '../router/routing-table'; export class InvokeMethodProvider implements Provider { constructor(@inject(RestBindings.Http.CONTEXT) protected context: Context) {} value(): InvokeMethod { - return async (route, args) => { - return await route.invokeHandler(this.context, args); - }; + return (route, args) => this.action(route, args); + } + + action(route: RouteEntry, args: OperationArgs): Promise { + return route.invokeHandler(this.context, args); } } diff --git a/packages/rest/src/providers/log-error-provider.ts b/packages/rest/src/providers/log-error-provider.ts index d9a89671aedc..17b580a7a354 100644 --- a/packages/rest/src/providers/log-error-provider.ts +++ b/packages/rest/src/providers/log-error-provider.ts @@ -3,21 +3,26 @@ // This file is licensed under the MIT License. // License text available at https://opensource.org/licenses/MIT -import {BoundValue, Provider} from '@loopback/context'; +import {Provider} from '@loopback/context'; import {ServerRequest} from '../'; +import {LogError} from '../internal-types'; -export class LogErrorProvider implements Provider { - value() { - return (err: Error, statusCode: number, req: ServerRequest) => { - if (statusCode >= 500) { - console.error( - 'Unhandled error in %s %s: %s %s', - req.method, - req.url, - statusCode, - err.stack || err, - ); - } - }; +export class LogErrorProvider implements Provider { + value(): LogError { + return (err, statusCode, req) => this.action(err, statusCode, req); + } + + action(err: Error, statusCode: number, req: ServerRequest) { + if (statusCode < 500) { + return; + } + + console.error( + 'Unhandled error in %s %s: %s %s', + req.method, + req.url, + statusCode, + err.stack || err, + ); } } diff --git a/packages/rest/src/providers/reject.ts b/packages/rest/src/providers/reject.ts index fd4cb1d3136f..6aee9ac512e2 100644 --- a/packages/rest/src/providers/reject.ts +++ b/packages/rest/src/providers/reject.ts @@ -4,24 +4,26 @@ // License text available at https://opensource.org/licenses/MIT import {LogError, Reject} from '../internal-types'; -import {inject} from '@loopback/context'; +import {inject, Provider} from '@loopback/context'; import {ServerResponse, ServerRequest} from 'http'; import {HttpError} from 'http-errors'; import {writeErrorToResponse} from '../writer'; import {RestBindings} from '../keys'; -export class RejectProvider { +export class RejectProvider implements Provider { constructor( @inject(RestBindings.SequenceActions.LOG_ERROR) protected logError: LogError, ) {} value(): Reject { - return (response: ServerResponse, request: ServerRequest, error: Error) => { - const err = error; - const statusCode = err.statusCode || err.status || 500; - writeErrorToResponse(response, err); - this.logError(error, statusCode, request); - }; + return (response, request, error) => this.action(response, request, error); + } + + action(response: ServerResponse, request: ServerRequest, error: Error) { + const err = error; + const statusCode = err.statusCode || err.status || 500; + writeErrorToResponse(response, err); + this.logError(error, statusCode, request); } }