fix: error message rewrite when it has only a getter (#3796)

lib/application.js

@@ -163,8 +163,17 @@ class Application extends EggApplication {
     graceful({
       server: [ server ],
       error: (err, throwErrorCount) => {
-        if (err.message) {
-          err.message += ' (uncaughtException throw ' + throwErrorCount + ' times on pid:' + process.pid + ')';
+        const originMessage = err.message;
+        if (originMessage) {
+          // shouldjs will override error property but only getter
+          // https://github.com/shouldjs/should.js/blob/889e22ebf19a06bc2747d24cf34b25cc00b37464/lib/assertion-error.js#L26
+          Object.defineProperty(err, 'message', {
+            get() {
+              return originMessage + ' (uncaughtException throw ' + throwErrorCount + ' times on pid:' + process.pid + ')';
+            },
+            configurable: true,
+            enumerable: false,
+          });
         }
         this.coreLogger.error(err);
       },

test/fixtures/apps/app-throw/app/router.js

@@ -8,6 +8,18 @@ module.exports = app => {
     }, 1);
   });
 
+  app.get('/throw-error-setter', function* () {
+    this.body = 'foo';
+    setTimeout(() => {
+      const err = new Error('abc');
+      Object.defineProperty(err, "message", {
+        get() { return 'abc' },
+        set: undefined
+      });
+      throw err;
+    }, 1);
+  });
+
   app.get('/throw-unhandledRejection', function* () {
     this.body = 'foo';
     new Promise((resolve, reject) => {

test/lib/application.test.js

@@ -134,6 +134,27 @@ describe('test/lib/application.test.js', () => {
     });
   });
 
+  describe('handle uncaughtException when error has only a getter', () => {
+    let app;
+    before(() => {
+      app = utils.cluster('apps/app-throw');
+      return app.ready();
+    });
+    after(() => app.close());
+
+    it('should handle uncaughtException and log it', async () => {
+      await app.httpRequest()
+        .get('/throw-error-setter')
+        .expect('foo')
+        .expect(200);
+
+      await sleep(1100);
+      const logfile = path.join(utils.getFilepath('apps/app-throw'), 'logs/app-throw/common-error.log');
+      const body = fs.readFileSync(logfile, 'utf8');
+      assert(body.includes('abc (uncaughtException throw 1 times on pid'));
+    });
+  });
+
   describe('warn confused configurations', () => {
     it('should warn if confused configurations exist', async () => {
       const app = utils.app('apps/confused-configuration');

test/lib/egg.test.js

@@ -272,7 +272,7 @@ describe('test/lib/egg.test.js', () => {
     after(() => app.close());
 
     // use it to record create coverage codes time
-    it('before: should cluster app ready', () => {
+    before('before: should cluster app ready', () => {
       app = utils.cluster('apps/app-throw');
       app.coverage(true);
       return app.ready();