From 8b550b8572dc8cdabc050476bce4a12ba35e76a0 Mon Sep 17 00:00:00 2001
From: Gcaufy <gcaufy@gmail.com>
Date: Sun, 1 Dec 2019 17:02:06 +0800
Subject: [PATCH] feat: added display-errors & display-sections option

---
 app.js                                        | 11 +++-
 config/config.default.js                      |  6 ++
 lib/error_view.js                             | 58 ++++++++++-------
 lib/onerror_page.mustache                     | 26 +++++---
 .../app/extend/context.js                     |  7 ++
 .../onerror-display-error/app/router.js       |  7 ++
 .../config/config.default.js                  |  6 ++
 .../onerror-display-error/package.json        |  3 +
 test/onerror.test.js                          | 64 +++++++++++++++++++
 9 files changed, 157 insertions(+), 31 deletions(-)
 create mode 100644 test/fixtures/onerror-display-error/app/extend/context.js
 create mode 100644 test/fixtures/onerror-display-error/app/router.js
 create mode 100644 test/fixtures/onerror-display-error/config/config.default.js
 create mode 100644 test/fixtures/onerror-display-error/package.json

diff --git a/app.js b/app.js
index ceb67cf..fdf9bb3 100644
--- a/app.js
+++ b/app.js
@@ -54,10 +54,19 @@ module.exports = app => {
         ? config.errorPageUrl(err, this)
         : config.errorPageUrl;
 
+      // If it's null, then use default isProd logic
+      let displayErrors = config.displayErrors === null ? !isProd(app) : config.displayErrors;
+
+      // It also can be a function
+      if (typeof displayErrors === 'function') {
+        displayErrors = displayErrors.call(app);
+      }
+
       // keep the real response status
       this.realStatus = status;
       // don't respond any error message in production env
-      if (isProd(app)) {
+      // if displayErrors set false, or unset, then use default logic
+      if (!displayErrors) {
         // 5xx
         if (status >= 500) {
           if (errorPageUrl) {
diff --git a/config/config.default.js b/config/config.default.js
index ddee862..ffdc109 100644
--- a/config/config.default.js
+++ b/config/config.default.js
@@ -11,4 +11,10 @@ exports.onerror = {
   appErrorFilter: null,
   // default template path
   templatePath: path.join(__dirname, '../lib/onerror_page.mustache'),
+  // Set displayErrors to true, to display erros.
+  // If it's not set, then use the default isProd logic for it.
+  displayErrors: null,
+  // Sections you want to show in error page.
+  // Default to show all sections.
+  displaySections: [ 'CodeFrames', 'Headers', 'Cookies', 'AppInfo' ],
 };
diff --git a/lib/error_view.js b/lib/error_view.js
index 19929ba..15446a3 100644
--- a/lib/error_view.js
+++ b/lib/error_view.js
@@ -23,6 +23,7 @@ class ErrorView {
     this.app = ctx.app;
     this.assets = new Map();
     this.viewTemplate = template;
+    this.displaySections = this.app.config.onerror.displaySections;
   }
 
   /**
@@ -41,8 +42,14 @@ class ErrorView {
     });
 
     data.request = this.serializeRequest();
-    data.appInfo = this.serializeAppInfo();
 
+    if (this.displaySections.indexOf('AppInfo') > -1) {
+      data.appInfo = this.serializeAppInfo();
+    }
+
+    this.displaySections.forEach(item => {
+      data['display' + item + 'Section'] = true;
+    });
     return this.complieView(this.viewTemplate, data);
   }
 
@@ -240,13 +247,17 @@ class ErrorView {
     if (code) {
       message = `${message} (code: ${code})`;
     }
-    return {
+
+    const data = {
       code,
       message,
       name: this.error.name,
       status: this.error.status,
-      frames: stack instanceof Array ? stack.filter(frame => frame.getFileName()).map(frameFomatter) : [],
     };
+    if (this.displaySections.indexOf('CodeFrames') > -1) {
+      data.frames = stack instanceof Array ? stack.filter(frame => frame.getFileName()).map(frameFomatter) : [];
+    }
+    return data;
   }
 
   /**
@@ -257,31 +268,34 @@ class ErrorView {
    * @memberOf ErrorView
    */
   serializeRequest() {
-    const headers = [];
-
-    Object.keys(this.request.headers).forEach(key => {
-      if (this._filterHeaders.includes(key)) {
-        return;
-      }
-      headers.push({
-        key,
-        value: this.request.headers[key],
-      });
-    });
-
-    const parsedCookies = cookie.parse(this.request.headers.cookie || '');
-    const cookies = Object.keys(parsedCookies).map(key => {
-      return { key, value: parsedCookies[key] };
-    });
 
-    return {
+    const data = {
       url: this.request.url,
       httpVersion: this.request.httpVersion,
       method: this.request.method,
       connection: this.request.headers.connection,
-      headers,
-      cookies,
     };
+
+    if (this.displaySections.indexOf('Headers') > -1) {
+      const headers = [];
+      Object.keys(this.request.headers).forEach(key => {
+        if (this._filterHeaders.includes(key)) {
+          return;
+        }
+        headers.push({
+          key,
+          value: this.request.headers[key],
+        });
+      });
+      data.headers = headers;
+    }
+    if (this.displaySections.indexOf('Cookies') > -1) {
+      const parsedCookies = cookie.parse(this.request.headers.cookie || '');
+      data.cookies = Object.keys(parsedCookies).map(key => {
+        return { key, value: parsedCookies[key] };
+      });
+    }
+    return data;
   }
 
   /**
diff --git a/lib/onerror_page.mustache b/lib/onerror_page.mustache
index cd520b9..38c722d 100644
--- a/lib/onerror_page.mustache
+++ b/lib/onerror_page.mustache
@@ -580,6 +580,7 @@
 
       <img class="error-logo" src="https://zos.alipayobjects.com/rmsportal/JFKAMfmPehWfhBPdCjrw.svg"/>
 
+      {{#displayCodeFramesSection}}
       <div class="error-frames">
         <div class="frame-preview is-hidden">
           <div id="frame-file"></div>
@@ -593,8 +594,8 @@
             <label for="frames-filter">Show all frames</label>
           </div>
 
+          {{#frames}}
           <div class="frames-list">
-            {{#frames}}
               {{index}}
               <div class="frame-row {{classes}}">
                 <div class="frame-row-filepath">
@@ -619,8 +620,8 @@
           </div>
         </div>
       </div>
+      {{/displayCodeFramesSection}}
     </section>
-
     <section class="request-details">
       <h2 class="request-title"> Request Details </h2>
       <div class="table">
@@ -645,25 +646,33 @@
         </div>
       </div>
 
+
+      {{#displayHeadersSection}}
       <h2 class="request-title"> Headers </h2>
       <div class="table">
-        {{#request.headers}}
+          {{#request.headers}}
           <div class="tr">
             <div class="td title">{{ key }}</div>
             <div class="td content">{{ value }}</div>
           </div>
-        {{/request.headers}}
+          {{/request.headers}}
       </div>
+      {{/displayHeadersSection}}
+
 
+      {{#displayCookiesSection}}
       <h2 class="request-title"> Cookies </h2>
       <div class="table">
-        {{#request.cookies}}
+          {{#request.cookies}}
           <div class="tr">
             <div class="td title">{{ key }}</div>
             <div class="td content">{{ value }}</div>
           </div>
-        {{/request.cookies}}
+          {{/request.cookies}}
       </div>
+      {{/displayCookiesSection}}
+
+      {{#displayAppInfoSection}}
       <h2 class="request-title"> AppInfo </h2>
       <div class="table">
         <div class="tr">
@@ -676,7 +685,8 @@
             <pre class="line-numbers"><code class="language-json">{{ appInfo.config }}</code></pre>
           </div>
         </div>
-      </<div>
+      </div>
+      {{/displayAppInfoSection}}
     </section>
 
     <script type="text/javascript">
@@ -751,4 +761,4 @@ Prism.languages.typescript=Prism.languages.extend("javascript",{keyword:/\b(as|a
     </script>
   </section>
 </body>
-</html>
\ No newline at end of file
+</html>
diff --git a/test/fixtures/onerror-display-error/app/extend/context.js b/test/fixtures/onerror-display-error/app/extend/context.js
new file mode 100644
index 0000000..4f9b893
--- /dev/null
+++ b/test/fixtures/onerror-display-error/app/extend/context.js
@@ -0,0 +1,7 @@
+'use strict';
+
+module.exports = {
+  get userId() {
+    throw new Error('you can`t get userId.');
+  },
+};
diff --git a/test/fixtures/onerror-display-error/app/router.js b/test/fixtures/onerror-display-error/app/router.js
new file mode 100644
index 0000000..31a0eb1
--- /dev/null
+++ b/test/fixtures/onerror-display-error/app/router.js
@@ -0,0 +1,7 @@
+'use strict';
+
+module.exports = app => {
+  app.get('/500', function* () {
+    this.throw(500, 'hi, this custom 500 page');
+  });
+};
diff --git a/test/fixtures/onerror-display-error/config/config.default.js b/test/fixtures/onerror-display-error/config/config.default.js
new file mode 100644
index 0000000..274ebd6
--- /dev/null
+++ b/test/fixtures/onerror-display-error/config/config.default.js
@@ -0,0 +1,6 @@
+'use strict';
+
+exports.keys = 'foo,bar';
+exports.onerror = {
+  displayErrors: process.env.SHOW_ERRORS - 1 === 0,
+};
diff --git a/test/fixtures/onerror-display-error/package.json b/test/fixtures/onerror-display-error/package.json
new file mode 100644
index 0000000..8c18ee3
--- /dev/null
+++ b/test/fixtures/onerror-display-error/package.json
@@ -0,0 +1,3 @@
+{
+  "name": "onerror-display-error"
+}
diff --git a/test/onerror.test.js b/test/onerror.test.js
index 239d27a..dd90e9a 100644
--- a/test/onerror.test.js
+++ b/test/onerror.test.js
@@ -382,6 +382,70 @@ describe('test/onerror.test.js', () => {
     });
   });
 
+  describe('onerror.display-error', () => {
+    let app;
+    before(() => {
+      process.env.SHOW_ERRORS = 1;
+      mm.env('prod');
+      mm.consoleLevel('NONE');
+      app = mm.app({
+        baseDir: 'onerror-display-error',
+      });
+      return app.ready();
+    });
+    after(() => {
+      delete process.env.SHOW_ERRORS;
+      app.close();
+    });
+
+    it('should 500 full html', () => {
+      return app.httpRequest()
+        .get('/500')
+        .expect(500)
+        .expect(/hi, this custom 500 page/);
+    });
+
+    it('should works as a function', () => {
+      mm(app.config.onerror, 'displayErrors', () => false);
+      return app.httpRequest()
+        .get('/500')
+        .expect(res => {
+          assert.equal(res.text.includes('Internal Server Error'), true);
+        })
+        .expect(500);
+    });
+
+    it('should no headers in 500 full html', () => {
+      mm(app.config.onerror, 'displaySections', [ 'CodeFrames', 'Cookies', 'AppInfo' ]);
+      mm(app.config.onerror, 'displayErrors', true);
+      return app.httpRequest()
+        .get('/500')
+        .expect(res => {
+          assert.equal(res.text.includes('<h2 class="request-title"> Cookies </h2>'), true);
+          assert.equal(res.text.includes('<div class="error-frames">'), true);
+          assert.equal(res.text.includes('<h2 class="request-title"> AppInfo </h2>'), true);
+          assert.equal(res.text.includes('<h2 class="request-title"> Headers </h2>'), false);
+        })
+        .expect(500)
+        .expect(/hi, this custom 500 page/);
+    });
+
+    it('should only have headers in 500 full html', () => {
+      mm(app.config.onerror, 'displaySections', [ 'Headers' ]);
+      mm(app.config.onerror, 'displayErrors', true);
+      return app.httpRequest()
+        .get('/500')
+        .expect(res => {
+          assert.equal(res.text.includes('<h2 class="request-title"> Cookies </h2>'), false);
+          assert.equal(res.text.includes('<div class="error-frames">'), false);
+          assert.equal(res.text.includes('<h2 class="request-title"> AppInfo </h2>'), false);
+          assert.equal(res.text.includes('<h2 class="request-title"> Headers </h2>'), true);
+        })
+        .expect(500)
+        .expect(/hi, this custom 500 page/);
+    });
+  });
+
   describe('appErrorFilter', () => {
     let app;
     before(() => {