Skip to content

Commit

Permalink
fix(rest): use strong-error-handler for writing errors to the respons…
Browse files Browse the repository at this point in the history
…e body
  • Loading branch information
shimks committed Aug 31, 2018
1 parent f3290cd commit ac011f8
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 68 deletions.
1 change: 1 addition & 0 deletions packages/rest/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
"lodash": "^4.17.5",
"openapi-schema-to-json-schema": "^2.1.0",
"path-to-regexp": "^2.2.0",
"strong-error-handler": "^3.2.0",
"validator": "^10.4.0"
},
"devDependencies": {
Expand Down
8 changes: 8 additions & 0 deletions packages/rest/src/keys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {

import {HttpProtocol} from '@loopback/http-server';
import * as https from 'https';
import {ErrorWriterOptions} from 'strong-error-handler';

/**
* RestServer-specific bindings
Expand Down Expand Up @@ -59,6 +60,13 @@ export namespace RestBindings {
* Internal binding key for http-handler
*/
export const HANDLER = BindingKey.create<HttpHandler>('rest.handler');
/**
* Binding key for setting and injecting Reject action's error handling
* options
*/
export const ERROR_WRITER_OPTIONS = BindingKey.create<ErrorWriterOptions>(
'rest.errorWriterOptions',
);

/**
* Binding key for setting and injecting an OpenAPI spec
Expand Down
18 changes: 4 additions & 14 deletions packages/rest/src/providers/reject.provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@
import {LogError, Reject, HandlerContext} from '../types';
import {inject, Provider} from '@loopback/context';
import {HttpError} from 'http-errors';
import {writeErrorToResponse} from '../writer';
import {RestBindings} from '../keys';

const debug = require('debug')('loopback:rest:reject');
import {writeErrorToResponse, ErrorWriterOptions} from 'strong-error-handler';

export class RejectProvider implements Provider<Reject> {
constructor(
@inject(RestBindings.SequenceActions.LOG_ERROR)
protected logError: LogError,
@inject(RestBindings.ERROR_WRITER_OPTIONS, {optional: true})
protected errorWriterOptions?: ErrorWriterOptions,
) {}

value(): Reject {
Expand All @@ -24,17 +24,7 @@ export class RejectProvider implements Provider<Reject> {
action({request, response}: HandlerContext, error: Error) {
const err = <HttpError>error;
const statusCode = err.statusCode || err.status || 500;
writeErrorToResponse(response, err);

// Always log the error in debug mode, even when the application
// has a custom error logger configured (e.g. in tests)
debug(
'HTTP request %s %s was rejected: %s %s',
request.method,
request.url,
statusCode,
err.stack || err,
);
writeErrorToResponse(err, request, response, this.errorWriterOptions);
this.logError(error, statusCode, request);
}
}
37 changes: 0 additions & 37 deletions packages/rest/src/writer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
// License text available at https://opensource.org/licenses/MIT

import {OperationRetval, Response} from './types';
import {HttpError} from 'http-errors';
import {Readable} from 'stream';

/**
Expand Down Expand Up @@ -51,39 +50,3 @@ export function writeResultToResponse(
}
response.end();
}

/**
* Write an error into the HTTP response
* @param response HTTP response
* @param error Error
*/
export function writeErrorToResponse(response: Response, error: Error) {
const e = <HttpError>error;
const statusCode = (response.statusCode = e.statusCode || e.status || 500);
if (e.headers) {
// Set response headers for the error
for (const h in e.headers) {
response.setHeader(h, e.headers[h]);
}
}
// Build an error object
const errObj: Partial<HttpError> = {
statusCode,
};
if (e.expose) {
// Expose other properties if the `expose` flag is set to `true`
for (const p in e) {
if (
p === 'headers' ||
p === 'expose' ||
p === 'status' ||
p === 'statusCode'
)
continue;
errObj[p] = e[p];
}
}
response.setHeader('Content-Type', 'application/json');
response.write(JSON.stringify(errObj));
response.end();
}
78 changes: 61 additions & 17 deletions packages/rest/test/integration/http-handler.integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
} from '../..';
import {ControllerSpec, get} from '@loopback/openapi-v3';
import {Context} from '@loopback/context';
import {Client, createClientForHandler} from '@loopback/testlab';
import {Client, createClientForHandler, expect} from '@loopback/testlab';
import * as HttpErrors from 'http-errors';
import {ParameterObject, RequestBodyObject} from '@loopback/openapi-v3-types';
import {anOpenApiSpec, anOperationSpec} from '@loopback/openapi-spec-builder';
Expand Down Expand Up @@ -248,9 +248,12 @@ describe('HttpHandler', () => {
.post('/show-body')
.send('key=value')
.expect(415, {
message:
'Content-type application/x-www-form-urlencoded is not supported.',
statusCode: 415,
error: {
message:
'Content-type application/x-www-form-urlencoded is not supported.',
name: 'UnsupportedMediaTypeError',
statusCode: 415,
},
});
});

Expand All @@ -260,7 +263,13 @@ describe('HttpHandler', () => {
.post('/show-body')
.set('content-type', 'application/json')
.send('malformed-json')
.expect(400, {statusCode: 400});
.expect(400, {
error: {
message: 'Unexpected token m in JSON at position 0',
name: 'SyntaxError',
statusCode: 400,
},
});
});

function givenBodyParamController() {
Expand Down Expand Up @@ -329,7 +338,7 @@ describe('HttpHandler', () => {
});

context('error handling', () => {
it('handles errors throws by controller constructor', () => {
it('handles errors thrown by controller constructor', () => {
const spec = anOpenApiSpec()
.withOperationReturningString('get', '/hello', 'greet')
.build();
Expand All @@ -344,7 +353,10 @@ describe('HttpHandler', () => {

logErrorsExcept(500);
return client.get('/hello').expect(500, {
statusCode: 500,
error: {
message: 'Internal Server Error',
statusCode: 500,
},
});
});

Expand All @@ -363,8 +375,11 @@ describe('HttpHandler', () => {
logErrorsExcept(404);

await client.get('/hello').expect(404, {
message: 'Controller method not found: TestController.unknownMethod',
statusCode: 404,
error: {
message: 'Controller method not found: TestController.unknownMethod',
name: 'NotFoundError',
statusCode: 404,
},
});
});

Expand All @@ -380,22 +395,20 @@ describe('HttpHandler', () => {
class TestController {
@get('/hello')
hello() {
const err = new HttpErrors.BadRequest('Bad hello');
err.headers = {'X-BAD-REQ': 'hello'};
throw err;
throw new HttpErrors.BadRequest('Bad hello');
}
}

givenControllerClass(TestController, spec);
logErrorsExcept(400);

await client
.get('/hello')
.expect('X-BAD-REQ', 'hello')
.expect(400, {
await client.get('/hello').expect(400, {
error: {
message: 'Bad hello',
name: 'BadRequestError',
statusCode: 400,
});
},
});
});

it('handles 500 error thrown from the method', async () => {
Expand All @@ -418,6 +431,37 @@ describe('HttpHandler', () => {
logErrorsExcept(500);

await client.get('/hello').expect(500, {
error: {
message: 'Internal Server Error',
statusCode: 500,
},
});
});

it('respects error handler options', async () => {
rootContext.bind(RestBindings.ERROR_WRITER_OPTIONS).to({debug: true});

const spec = anOpenApiSpec()
.withOperation(
'get',
'/hello',
anOperationSpec().withOperationName('hello'),
)
.build();

class TestController {
@get('/hello')
hello() {
throw new HttpErrors.InternalServerError('Bad hello');
}
}

givenControllerClass(TestController, spec);
logErrorsExcept(500);

const response = await client.get('/hello').expect(500);
expect(response.body.error).to.containDeep({
message: 'Bad hello',
statusCode: 500,
});
});
Expand Down

0 comments on commit ac011f8

Please sign in to comment.