Merge pull request #90 from strongloop/root-property

feat: Add options.rootProperty

.travis.yml

@@ -1,6 +1,6 @@
 sudo: false
 language: node_js
 node_js:
-  - "8"
   - "10"
   - "12"
+  - "14"

README.md

@@ -14,9 +14,9 @@ In debug mode, `strong-error-handler` returns full error stack traces and intern
 
 ## Supported versions
 
-Current|Long Term Support|Maintenance
+| Current | Long Term Support | Maintenance |
-:-:|:-:|:-:
+| :-----: | :---------------: | :---------: |
-3.x|2.x|1.x
+|   4.x   |        3.x        |     2.x     |
 
 Learn more about our LTS plan in [docs](http://loopback.io/doc/en/contrib/Long-term-support.html).
 
@@ -108,13 +108,14 @@ The content type of the response depends on the request's `Accepts` header.
 
 ## Options
 
-| Option | Type | Default | Description |
+| Option               | Type                      | Default   | Description                                                                                                                                                                                                                                                                                                                                               |
-| ---- | ---- | ---- | ---- |
+| -------------------- | ------------------------- | --------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| debug | Boolean    | `false` | If `true`, HTTP responses include all error properties, including sensitive data such as file paths, URLs and stack traces. See [Example output](#example) below. |
+| debug                | Boolean    | `false`   | If `true`, HTTP responses include all error properties, including sensitive data such as file paths, URLs and stack traces. See [Example output](#example) below.                                                                                                                                                                                         |
-| log | Boolean | `true` |  If `true`, all errors are printed via `console.error`, including an array of fields (custom error properties) that are safe to include in response messages (both 4xx and 5xx). <br/> If `false`, sends only the error back in the response. |
+| log                  | Boolean                   | `true`    | If `true`, all errors are printed via `console.error`, including an array of fields (custom error properties) that are safe to include in response messages (both 4xx and 5xx). <br/> If `false`, sends only the error back in the response.                                                                                                              |
-| safeFields | [String] | `[]` |  Specifies property names on errors that are allowed to be passed through in 4xx and 5xx responses. See [Safe error fields](#safe-error-fields) below. |
+| safeFields           | [String]                  | `[]`      | Specifies property names on errors that are allowed to be passed through in 4xx and 5xx responses. See [Safe error fields](#safe-error-fields) below.                                                                                                                                                                                                     |
-| defaultType | String | `"json"` | Specify the default response content type to use when the client does not provide any Accepts header.
+| defaultType          | String                    | `"json"`  | Specifies the default response content type to use when the client does not provide any Accepts header.                                                                                                                                                                                                                                                   |
-| negotiateContentType | Boolean | true | Negotiate the response content type via Accepts request header. When disabled, strong-error-handler will always use the default content type when producing responses. Disabling content type negotiation is useful if you want to see JSON-formatted error responses in browsers, because browsers usually prefer HTML and XML over other content types.
+| rootProperty         | String or false           | `"error"` | Specifies the root property name for json or xml. If the value is set to `false`, no wrapper will be added to the json object. The false value is ignored by XML as a root element is always required.                                                                                                                                                    |
+| negotiateContentType | Boolean                   | true      | Negotiate the response content type via Accepts request header. When disabled, strong-error-handler will always use the default content type when producing responses. Disabling content type negotiation is useful if you want to see JSON-formatted error responses in browsers, because browsers usually prefer HTML and XML over other content types. |
 
 ### Customizing log format
 
@@ -125,7 +126,7 @@ For example, in an Express application:
 
 ```js
 app.use(myErrorLogger());
-app.use(errorHandler({ log: false }));
+app.use(errorHandler({log: false}));
 ```
 
 In general, add `strong-error-handler` as the last middleware function, just before calling `app.listen()`.

index.d.ts

@@ -53,6 +53,7 @@ declare namespace errorHandlerFactory {
     safeFields?: string[];
     defaultType?: string;
     negotiateContentType?: boolean;
+    rootProperty?: string | false;
   }
 
   /**

lib/handler.js

@@ -10,7 +10,6 @@ const SG = require('strong-globalize');
 SG.SetRootDir(path.resolve(__dirname, '..'));
 const buildResponseData = require('./data-builder');
 const debug = require('debug')('strong-error-handler');
-const format = require('util').format;
 const logToConsole = require('./logger');
 const negotiateContentProducer = require('./content-negotiation');
 
@@ -50,7 +49,7 @@ function writeErrorToResponse(err, req, res, options) {
 
   options = options || {};
 
-  if (res._header) {
+  if (res.headersSent) {
     debug('Response was already sent, closing the underlying connection');
     return req.socket.destroy();
   }
@@ -66,7 +65,7 @@ function writeErrorToResponse(err, req, res, options) {
   res.statusCode = data.statusCode;
 
   const sendResponse = negotiateContentProducer(req, warn, options);
-  sendResponse(res, data);
+  sendResponse(res, data, options);
 
   function warn(msg) {
     res.header('X-Warning', msg);

lib/send-html.js

@@ -17,10 +17,10 @@ const compiledTemplates = {
 module.exports = sendHtml;
 
 function sendHtml(res, data, options) {
-  const toRender = {options: {}, data: data};
+  const toRender = {options, data};
   // TODO: ability to call non-default template functions from options
   const body = compiledTemplates.default(toRender);
-  sendReponse(res, body);
+  sendResponse(res, body);
 }
 
 /**
@@ -41,7 +41,7 @@ function loadDefaultTemplates() {
   return compileTemplate(defaultTemplate);
 }
 
-function sendReponse(res, body) {
+function sendResponse(res, body) {
   res.setHeader('Content-Type', 'text/html; charset=utf-8');
   res.end(body);
 }

lib/send-json.js

@@ -7,8 +7,14 @@
 
 const safeStringify = require('fast-safe-stringify');
 
-module.exports = function sendJson(res, data) {
+module.exports = function sendJson(res, data, options) {
-  const content = safeStringify({error: data});
+  options = options || {};
+  // Set `options.rootProperty` to not wrap the data into an `error` object
+  const err = options.rootProperty === false ? data : {
+    // Use `options.rootProperty`, if not set, default to `error`
+    [options.rootProperty || 'error']: data,
+  };
+  const content = safeStringify(err);
   res.setHeader('Content-Type', 'application/json; charset=utf-8');
   res.end(content, 'utf-8');
 };

lib/send-xml.js

@@ -7,8 +7,12 @@
 
 const js2xmlparser = require('js2xmlparser');
 
-module.exports = function sendXml(res, data) {
+module.exports = function sendXml(res, data, options) {
-  const content = js2xmlparser.parse('error', data);
+  options = options || {};
+  // Xml always requires a root element.
+  // `options.rootProperty === false` is not honored
+  const root = options.rootProperty || 'error';
+  const content = js2xmlparser.parse(root, data);
   res.setHeader('Content-Type', 'text/xml; charset=utf-8');
   res.end(content, 'utf-8');
 };

package.json

@@ -4,7 +4,7 @@
   "license": "MIT",
   "version": "3.4.0",
   "engines": {
-    "node": ">=8"
+    "node": ">=10"
   },
   "repository": {
     "type": "git",
@@ -20,18 +20,18 @@
     "@types/express": "^4.16.0",
     "accepts": "^1.3.3",
     "debug": "^4.1.1",
-    "ejs": "^2.6.1",
+    "ejs": "^3.1.3",
     "fast-safe-stringify": "^2.0.6",
     "http-status": "^1.1.2",
     "js2xmlparser": "^4.0.0",
-    "strong-globalize": "^5.0.2"
+    "strong-globalize": "^6.0.1"
   },
   "devDependencies": {
     "chai": "^4.1.2",
-    "eslint": "^6.5.1",
+    "eslint": "^7.0.0",
     "eslint-config-loopback": "^13.1.0",
     "express": "^4.16.3",
-    "mocha": "^6.2.1",
+    "mocha": "^7.1.2",
     "supertest": "^4.0.2"
   },
   "browser": {

test/handler.test.js

@@ -78,7 +78,7 @@ describe('strong-error-handler', function() {
       request.get('/').expect(
         507,
         {error: {statusCode: 507, message: 'Insufficient Storage'}},
-        done
+        done,
       );
     });
   });
@@ -146,7 +146,7 @@ describe('strong-error-handler', function() {
     it('handles array argument', function(done) {
       givenErrorHandlerForError(
         [new TypeError('ERR1'), new Error('ERR2')],
-        {log: true}
+        {log: true},
       );
 
       request.get('/api').end(function(err) {
@@ -541,6 +541,32 @@ describe('strong-error-handler', function() {
       });
     });
 
+    it('honors rootProperty', function(done) {
+      givenErrorHandlerForError('Error Message', {rootProperty: 'data'});
+      requestJson().expect(500).end(function(err, res) {
+        if (err) return done(err);
+
+        expect(res.body.data).to.eql({
+          statusCode: 500,
+          message: 'Internal Server Error',
+        });
+        done();
+      });
+    });
+
+    it('honors rootProperty=false', function(done) {
+      givenErrorHandlerForError('Error Message', {rootProperty: false});
+      requestJson().expect(500).end(function(err, res) {
+        if (err) return done(err);
+
+        expect(res.body).to.eql({
+          statusCode: 500,
+          message: 'Internal Server Error',
+        });
+        done();
+      });
+    });
+
     function requestJson(url) {
       return request.get(url || '/')
         .set('Accept', 'text/plain')
@@ -581,10 +607,10 @@ describe('strong-error-handler', function() {
             expect(res.statusCode).to.eql(404);
             const body = res.error.text;
             expect(body).to.match(
-              /<title>Error&lt;img onerror=alert\(1\) src=a&gt;<\/title>/
+              /<title>Error&lt;img onerror=alert\(1\) src=a&gt;<\/title>/,
             );
             expect(body).to.match(
-              /with id &lt;img onerror=alert\(1\) src=a&gt; found for Model/
+              /with id &lt;img onerror=alert\(1\) src=a&gt; found for Model/,
             );
             done();
           });
@@ -602,7 +628,7 @@ describe('strong-error-handler', function() {
           .expect(/<title>ErrorWithProps<\/title>/)
           .expect(
             /500(.*?)a test error message&lt;img onerror=alert\(1\) src=a&gt;/,
-            done
+            done,
           );
       });
 
@@ -696,7 +722,7 @@ describe('strong-error-handler', function() {
           expect(body).to.not.match(/<extra>sensitive data<\/extra>/);
           expect(body).to.match(/<name>ValidationError<\/name>/);
           expect(body).to.match(
-            /<message>The model instance is not valid.<\/message>/
+            /<message>The model instance is not valid.<\/message>/,
           );
           done();
         });
@@ -728,6 +754,54 @@ describe('strong-error-handler', function() {
         });
     });
 
+    it('honors options.rootProperty', function(done) {
+      const error = new ErrorWithProps({
+        name: 'ValidationError',
+        message: 'The model instance is not valid.',
+        statusCode: 422,
+        details: 'some details',
+        extra: 'sensitive data',
+      });
+      givenErrorHandlerForError(error, {rootProperty: 'myRoot'});
+      requestXML()
+        .end(function(err, res) {
+          expect(res.statusCode).to.eql(422);
+          const body = res.error.text;
+          expect(body).to.match(/<myRoot>/);
+          expect(body).to.match(/<details>some details<\/details>/);
+          expect(body).to.not.match(/<extra>sensitive data<\/extra>/);
+          expect(body).to.match(/<name>ValidationError<\/name>/);
+          expect(body).to.match(
+            /<message>The model instance is not valid.<\/message>/,
+          );
+          done();
+        });
+    });
+
+    it('ignores options.rootProperty = false', function(done) {
+      const error = new ErrorWithProps({
+        name: 'ValidationError',
+        message: 'The model instance is not valid.',
+        statusCode: 422,
+        details: 'some details',
+        extra: 'sensitive data',
+      });
+      givenErrorHandlerForError(error, {rootProperty: false});
+      requestXML()
+        .end(function(err, res) {
+          expect(res.statusCode).to.eql(422);
+          const body = res.error.text;
+          expect(body).to.match(/<error>/);
+          expect(body).to.match(/<details>some details<\/details>/);
+          expect(body).to.not.match(/<extra>sensitive data<\/extra>/);
+          expect(body).to.match(/<name>ValidationError<\/name>/);
+          expect(body).to.match(
+            /<message>The model instance is not valid.<\/message>/,
+          );
+          done();
+        });
+    });
+
     function requestXML(url) {
       return request.get(url || '/')
         .set('Accept', 'text/xml')

views/default-error.ejs

@@ -2,7 +2,7 @@
   <head>
     <meta charset='utf-8'>
     <title><%= data.name || data.message %></title>
-    <style><%- include style.css %></style>
+    <style><%- include('style.css') %></style>
   </head>
   <body>
     <div id="wrapper">