feat(rest): Improve http error handling

Error are now sent back as json object per HttpError structure
Error properties are controlled by `expose`
Only log http 5xx errors to console

Author: raymondfeng

package.json

@@ -42,7 +42,7 @@
     "build:current": "lerna run --loglevel=silent build:current",
     "pretest": "npm run build:current",
     "test": "node packages/build/bin/run-nyc npm run mocha",
-    "mocha": "node packages/build/bin/select-dist mocha --exit --opts test/mocha.opts \"packages/*/DIST/test/**/*.js\"",
+    "mocha": "node packages/build/bin/select-dist mocha --opts test/mocha.opts \"packages/*/DIST/test/**/*.js\"",
     "posttest": "npm run lint"
   },
   "config": {

packages/rest/src/providers/log-error-provider.ts

@@ -9,13 +9,15 @@ import {ServerRequest} from '../';
 export class LogErrorProvider implements Provider<BoundValue> {
   value() {
     return (err: Error, statusCode: number, req: ServerRequest) => {
-      console.error(
-        'Unhandled error in %s %s: %s %s',
-        req.method,
-        req.url,
-        statusCode,
-        err.stack || err,
-      );
+      if (statusCode >= 500) {
+        console.error(
+          'Unhandled error in %s %s: %s %s',
+          req.method,
+          req.url,
+          statusCode,
+          err.stack || err,
+        );
+      }
     };
   }
 }

packages/rest/src/providers/reject.ts

@@ -7,6 +7,7 @@ import {LogError, Reject} from '../internal-types';
 import {inject} from '@loopback/context';
 import {ServerResponse, ServerRequest} from 'http';
 import {HttpError} from 'http-errors';
+import {writeErrorToResponse} from '../writer';
 import {RestBindings} from '../keys';
 
 export class RejectProvider {
@@ -17,11 +18,9 @@ export class RejectProvider {
 
   value(): Reject {
     return (response: ServerResponse, request: ServerRequest, error: Error) => {
-      const err = error as HttpError;
+      const err = <HttpError>error;
       const statusCode = err.statusCode || err.status || 500;
-      response.statusCode = statusCode;
-      response.end();
-
+      writeErrorToResponse(response, err);
       this.logError(error, statusCode, request);
     };
   }

packages/rest/src/writer.ts

@@ -5,6 +5,7 @@
 
 import {ServerResponse as Response} from 'http';
 import {OperationRetval} from './internal-types';
+import {HttpError} from 'http-errors';
 /**
  * Writes the result from Application controller method
  * into the HTTP response
@@ -36,3 +37,39 @@ 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();
+}

packages/rest/test/integration/http-handler.integration.ts

@@ -11,9 +11,11 @@ import {
   ControllerSpec,
   parseOperationArgs,
   RestBindings,
+  get,
 } from '../..';
 import {Context} from '@loopback/context';
 import {Client, createClientForHandler} from '@loopback/testlab';
+import * as HttpErrors from 'http-errors';
 import {ParameterObject} from '@loopback/openapi-spec';
 import {anOpenApiSpec, anOperationSpec} from '@loopback/openapi-spec-builder';
 import {
@@ -308,7 +310,11 @@ describe('HttpHandler', () => {
       return client
         .post('/show-body')
         .send('key=value')
-        .expect(415);
+        .expect(415, {
+          message:
+            'Content-type application/x-www-form-urlencoded is not supported.',
+          statusCode: 415,
+        });
     });
 
     it('returns 400 for malformed JSON body', () => {
@@ -317,7 +323,7 @@ describe('HttpHandler', () => {
         .post('/show-body')
         .set('content-type', 'application/json')
         .send('malformed-json')
-        .expect(400);
+        .expect(400, {statusCode: 400});
     });
 
     function givenBodyParamController() {
@@ -396,7 +402,9 @@ describe('HttpHandler', () => {
       givenControllerClass(ThrowingController, spec);
 
       logErrorsExcept(500);
-      return client.get('/hello').expect(500);
+      return client.get('/hello').expect(500, {
+        statusCode: 500,
+      });
     });
 
     it('handles invocation of an unknown method', async () => {
@@ -413,7 +421,64 @@ describe('HttpHandler', () => {
       givenControllerClass(TestController, spec);
       logErrorsExcept(404);
 
-      await client.get('/hello').expect(404);
+      await client.get('/hello').expect(404, {
+        message: 'Controller method not found: TestController.unknownMethod',
+        statusCode: 404,
+      });
+    });
+
+    it('handles errors thrown from the method', async () => {
+      const spec = anOpenApiSpec()
+        .withOperation(
+          'get',
+          '/hello',
+          anOperationSpec().withOperationName('hello'),
+        )
+        .build();
+
+      class TestController {
+        @get('/hello')
+        hello() {
+          const err = new HttpErrors.BadRequest('Bad hello');
+          err.headers = {'X-BAD-REQ': 'hello'};
+          throw err;
+        }
+      }
+
+      givenControllerClass(TestController, spec);
+      logErrorsExcept(400);
+
+      await client
+        .get('/hello')
+        .expect('X-BAD-REQ', 'hello')
+        .expect(400, {
+          message: 'Bad hello',
+          statusCode: 400,
+        });
+    });
+
+    it('handles 500 error thrown from the method', async () => {
+      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(400);
+
+      await client.get('/hello').expect(500, {
+        statusCode: 500,
+      });
     });
   });
 

test/mocha.opts

@@ -1 +1,2 @@
 --recursive
+--exit