From 74565e2e9fc6dee0e97d95eeebd721610ee48872 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Fri, 2 Jun 2017 15:06:32 +0200 Subject: [PATCH] Move logError to app-level binding --- packages/core/src/application.ts | 7 ++++ packages/core/src/http-handler.ts | 10 ++---- .../integration/http-handler.integration.ts | 17 +++++++--- .../test/unit/application/application.test.ts | 34 +++++++++++++++++++ 4 files changed, 56 insertions(+), 12 deletions(-) create mode 100644 packages/core/test/unit/application/application.test.ts diff --git a/packages/core/src/application.ts b/packages/core/src/application.ts index b5968f4218d8..61d0fe0bedaa 100644 --- a/packages/core/src/application.ts +++ b/packages/core/src/application.ts @@ -59,6 +59,8 @@ export class Application extends Context { this.handleHttp = (req: ServerRequest, res: ServerResponse) => this._handleHttpRequest(req, res); + + this.bind('logError').to(this._logError.bind(this)); } protected _handleHttpRequest(request: ServerRequest, response: ServerResponse) { @@ -100,6 +102,11 @@ export class Application extends Context { public controller(controllerCtor: Constructor): Binding { return this.bind('controllers.' + controllerCtor.name).toClass(controllerCtor); } + + protected _logError(err: Error, statusCode: number, req: ServerRequest): void { + console.error('Unhandled error in %s %s: %s %s', + req.method, req.url, statusCode, err.stack || err); + } } export interface AppConfig { diff --git a/packages/core/src/http-handler.ts b/packages/core/src/http-handler.ts index 727b3f00843c..3c79b19e010d 100644 --- a/packages/core/src/http-handler.ts +++ b/packages/core/src/http-handler.ts @@ -35,11 +35,12 @@ export class HttpHandler { this._bindFindRoute(requestContext); this._bindInvokeMethod(requestContext); + // TODO(bajtos) instantiate the Sequence via ctx.get() const findRoute = await requestContext.get('findRoute'); const invokeMethod = await requestContext.get('invokeMethod'); + const logError = await requestContext.get('logError'); + const sequence = new Sequence(findRoute, invokeMethod, logError); - // TODO(bajtos) instantiate the Sequence via ctx.get() - const sequence = new Sequence(findRoute, invokeMethod, this.logError.bind(this)); return sequence.run(parsedRequest, response); } @@ -79,9 +80,4 @@ export class HttpHandler { }; }); } - - logError(err: Error, statusCode: number, req: ServerRequest): void { - console.error('Unhandled error in %s %s: %s %s', - req.method, req.url, statusCode, err.stack || err); - } } diff --git a/packages/core/test/integration/http-handler.integration.ts b/packages/core/test/integration/http-handler.integration.ts index f541367b5636..3551ca8e3ac9 100644 --- a/packages/core/test/integration/http-handler.integration.ts +++ b/packages/core/test/integration/http-handler.integration.ts @@ -3,7 +3,7 @@ // This file is licensed under the MIT License. // License text available at https://opensource.org/licenses/MIT -import {HttpHandler, Sequence} from '../..'; +import {HttpHandler, Sequence, ServerRequest} from '../..'; import {Context} from '@loopback/context'; import {expect, Client, createClientForHandler} from '@loopback/testlab'; import {OpenApiSpec, ParameterObject} from '@loopback/openapi-spec'; @@ -374,6 +374,12 @@ context('with an operation echoing a string parameter from query', () => { let handler: HttpHandler; function givenHandler() { rootContext = new Context(); + rootContext.bind('logError').to(logger); + function logger(err: Error, statusCode: number, req: ServerRequest) { + console.error('Unhandled error in %s %s: %s %s', + req.method, req.url, statusCode, err.stack || err); + } + handler = new HttpHandler(rootContext); } @@ -388,12 +394,13 @@ context('with an operation echoing a string parameter from query', () => { } function logErrorsExcept(ignoreStatusCode: number) { - // TODO(bajtos) Rework this code to customize the logger via rootContext.bind() - const oldLogger = handler.logError; - handler.logError = (err, statusCode, req) => { + const oldLogger: Function = rootContext.getSync('logError'); + rootContext.bind('logError').to(conditionalLogger); + + function conditionalLogger(err: Error, statusCode: number, req: ServerRequest) { if (statusCode === ignoreStatusCode) return; // tslint:disable-next-line:no-invalid-this oldLogger.apply(this, arguments); - }; + } } }); diff --git a/packages/core/test/unit/application/application.test.ts b/packages/core/test/unit/application/application.test.ts new file mode 100644 index 000000000000..1e9df29a41a4 --- /dev/null +++ b/packages/core/test/unit/application/application.test.ts @@ -0,0 +1,34 @@ +// Copyright IBM Corp. 2013,2017. All Rights Reserved. +// Node module: @loopback/core +// This file is licensed under the MIT License. +// License text available at https://opensource.org/licenses/MIT + +import {expect, ShotRequest} from '@loopback/testlab'; +import {Application, ServerRequest} from '../../..'; + +describe('Application', () => { + describe('"logError" binding', () => { + it('provides a default', async () => { + const app = new Application(); + const logError = await app.get('logError'); + expect(logError.length).to.equal(3); // (err, statusCode, request) + }); + + it('can be customized by overriding Application._logError() method', async () => { + let lastLog: string = 'logError() was not called'; + + class MyApp extends Application { + protected _logError(err: Error, statusCode: number, request: ServerRequest) { + lastLog = `${request.url} ${statusCode} ${err.message}`; + } + } + + const app = new MyApp(); + const logError = await app.get('logError'); + logError(new Error('test-error'), 400, new ShotRequest({url: '/'})); + + expect(lastLog).to.equal('/ 400 test-error'); + }); + }); +}); +