From 57916a55364faf26b0de48c25198188487404832 Mon Sep 17 00:00:00 2001 From: popomore Date: Tue, 21 Aug 2018 21:08:54 +0800 Subject: [PATCH 01/10] feat: add biz handler There is three case: 1. If it's an egg error, it will format response 1. If it's an egg exception, it will format response and log an error 1. if it's a normal error, it will be throw that catched by onerror handler --- app.js | 2 ++ app/middleware/onerror_biz_handler.js | 47 +++++++++++++++++++++++++++ config/config.default.js | 2 ++ 3 files changed, 51 insertions(+) create mode 100644 app/middleware/onerror_biz_handler.js diff --git a/app.js b/app.js index ceb67cf..29ff3b0 100644 --- a/app.js +++ b/app.js @@ -12,6 +12,8 @@ const { } = require('./lib/utils'); module.exports = app => { + app.config.coreMiddleware.push('onerrorBizHandler'); + // logging error const config = app.config.onerror; const viewTemplate = fs.readFileSync(config.templatePath, 'utf8'); diff --git a/app/middleware/onerror_biz_handler.js b/app/middleware/onerror_biz_handler.js new file mode 100644 index 0000000..1b78663 --- /dev/null +++ b/app/middleware/onerror_biz_handler.js @@ -0,0 +1,47 @@ +'use strict'; + +const { EggError, ErrorType } = require('egg-errors'); + +module.exports = (_, app) => { + const formatError = app.config.onerror.formatError || defaultFormat; + + return async function onerrorBizHandler(ctx, next) { + try { + await next(); + } catch (err) { + const type = EggError.getType(err); + switch (type) { + case ErrorType.ERROR: + await handleError(ctx, err); + break; + + case ErrorType.EXCEPTION: + await handleException(ctx, err); + break; + + case ErrorType.BUILTIN: + default: + throw err; + } + } + }; + + async function handleError(ctx, err) { + ctx.status = err.status || 500; + ctx.body = formatError(err); + } + + async function handleException(ctx, err) { + ctx.logger.error(err); + err.status = 500; + ctx.body = formatError(err); + } + +}; + +function defaultFormat(err) { + return { + code: err.code, + message: err.message, + }; +} diff --git a/config/config.default.js b/config/config.default.js index ddee862..4999370 100644 --- a/config/config.default.js +++ b/config/config.default.js @@ -9,6 +9,8 @@ exports.onerror = { // If `appErrorFilter` return false, egg-onerror won't log this error. // You can logging in `appErrorFilter` and return false to override the default error logging. appErrorFilter: null, + // normalize your error response from error object + formatError: null, // default template path templatePath: path.join(__dirname, '../lib/onerror_page.mustache'), }; From f0cd1b450358878e00efa3b1d6fe40668990a3ec Mon Sep 17 00:00:00 2001 From: popomore Date: Wed, 22 Aug 2018 23:44:41 +0800 Subject: [PATCH 02/10] f --- app/middleware/onerror_biz_handler.js | 42 +++++++++++++++++---------- config/config.default.js | 13 +++++++-- 2 files changed, 37 insertions(+), 18 deletions(-) diff --git a/app/middleware/onerror_biz_handler.js b/app/middleware/onerror_biz_handler.js index 1b78663..3b342b6 100644 --- a/app/middleware/onerror_biz_handler.js +++ b/app/middleware/onerror_biz_handler.js @@ -1,41 +1,51 @@ 'use strict'; const { EggError, ErrorType } = require('egg-errors'); +const { accepts } = require('./lib/utils'); module.exports = (_, app) => { - const formatError = app.config.onerror.formatError || defaultFormat; + const biz = app.config.onerror.biz; return async function onerrorBizHandler(ctx, next) { try { await next(); } catch (err) { + const fn = app.config.onerror.accepts || accepts; + const contentType = fn(ctx); + const type = EggError.getType(err); switch (type) { - case ErrorType.ERROR: - await handleError(ctx, err); + case ErrorType.ERROR: { + ctx.status = err.status || 500; + ctx.body = format(err, contentType); break; + } - case ErrorType.EXCEPTION: - await handleException(ctx, err); + case ErrorType.EXCEPTION: { + ctx.logger.error(err); + err.status = 500; + ctx.body = format(err, contentType); break; + } case ErrorType.BUILTIN: default: throw err; } } - }; - - async function handleError(ctx, err) { - ctx.status = err.status || 500; - ctx.body = formatError(err); - } - async function handleException(ctx, err) { - ctx.logger.error(err); - err.status = 500; - ctx.body = formatError(err); - } + function format(err, contentType) { + if (contentType === 'json' && biz.formatJSON) { + ctx.body = biz.formatJSON(err); + if (contentType === 'text' && biz.formatText) { + ctx.body = biz.formatText(err); + if (contentType === 'html' && biz.formatHtml) { + ctx.body = biz.formatHtml(err); + } else { + ctx.body = defaultFormat(err); + } + } + }; }; diff --git a/config/config.default.js b/config/config.default.js index 4999370..3dda053 100644 --- a/config/config.default.js +++ b/config/config.default.js @@ -9,8 +9,17 @@ exports.onerror = { // If `appErrorFilter` return false, egg-onerror won't log this error. // You can logging in `appErrorFilter` and return false to override the default error logging. appErrorFilter: null, - // normalize your error response from error object - formatError: null, // default template path templatePath: path.join(__dirname, '../lib/onerror_page.mustache'), + // normalize your error response from error object + biz: { + formatJSON: null, + formatText: null, + formatHtml: null, + }, + system: { + formatJSON: null, + formatText: null, + formatHtml: null, + }, }; From 71696904a9b83784e71b450e74c09828982c24bf Mon Sep 17 00:00:00 2001 From: popomore Date: Fri, 24 Aug 2018 12:48:37 +0800 Subject: [PATCH 03/10] f --- app.js | 4 +- app/middleware/onerror_biz_handler.js | 30 ++++++--------- config/config.default.js | 7 +--- package.json | 1 + .../onerror-biz/app/controller/home.js | 22 +++++++++++ test/fixtures/onerror-biz/app/router.js | 7 ++++ .../onerror-biz/config/config.default.js | 37 +++++++++++++++++++ test/fixtures/onerror-biz/package.json | 3 ++ test/onerror.test.js | 24 ++++++++++++ 9 files changed, 109 insertions(+), 26 deletions(-) create mode 100644 test/fixtures/onerror-biz/app/controller/home.js create mode 100644 test/fixtures/onerror-biz/app/router.js create mode 100644 test/fixtures/onerror-biz/config/config.default.js create mode 100644 test/fixtures/onerror-biz/package.json diff --git a/app.js b/app.js index 29ff3b0..478975a 100644 --- a/app.js +++ b/app.js @@ -45,9 +45,9 @@ module.exports = app => { const errorOptions = { // support customize accepts function - accepts() { + accepts(...args) { const fn = config.accepts || accepts; - return fn(this); + return fn(this, ...args); }, html(err) { diff --git a/app/middleware/onerror_biz_handler.js b/app/middleware/onerror_biz_handler.js index 3b342b6..e67af07 100644 --- a/app/middleware/onerror_biz_handler.js +++ b/app/middleware/onerror_biz_handler.js @@ -1,19 +1,20 @@ 'use strict'; const { EggError, ErrorType } = require('egg-errors'); -const { accepts } = require('./lib/utils'); +const { accepts } = require('../../lib/utils'); module.exports = (_, app) => { - const biz = app.config.onerror.biz; + const application = app.config.onerror.application; return async function onerrorBizHandler(ctx, next) { try { await next(); } catch (err) { const fn = app.config.onerror.accepts || accepts; - const contentType = fn(ctx); - + const contentType = fn(ctx, 'html', 'text', 'json'); + console.info(contentType); const type = EggError.getType(err); + console.log(type); switch (type) { case ErrorType.ERROR: { ctx.status = err.status || 500; @@ -35,23 +36,16 @@ module.exports = (_, app) => { } function format(err, contentType) { - if (contentType === 'json' && biz.formatJSON) { - ctx.body = biz.formatJSON(err); - if (contentType === 'text' && biz.formatText) { - ctx.body = biz.formatText(err); - if (contentType === 'html' && biz.formatHtml) { - ctx.body = biz.formatHtml(err); + if (contentType === 'json' && application.formatJSON) { + ctx.body = application.formatJSON(err); + } else if (contentType === 'text' && application.formatText) { + ctx.body = application.formatText(err); + } else if (contentType === 'html' && application.formatHtml) { + ctx.body = application.formatHtml(err); } else { - ctx.body = defaultFormat(err); + throw err; } } }; }; - -function defaultFormat(err) { - return { - code: err.code, - message: err.message, - }; -} diff --git a/config/config.default.js b/config/config.default.js index 3dda053..f6eabb4 100644 --- a/config/config.default.js +++ b/config/config.default.js @@ -12,12 +12,7 @@ exports.onerror = { // default template path templatePath: path.join(__dirname, '../lib/onerror_page.mustache'), // normalize your error response from error object - biz: { - formatJSON: null, - formatText: null, - formatHtml: null, - }, - system: { + application: { formatJSON: null, formatText: null, formatHtml: null, diff --git a/package.json b/package.json index 9a3ba98..b8a3cd5 100644 --- a/package.json +++ b/package.json @@ -25,6 +25,7 @@ ], "dependencies": { "cookie": "^0.3.1", + "egg-errors": "^1.0.1", "koa-onerror": "^4.0.0", "mustache": "^2.3.0", "stack-trace": "^0.0.10" diff --git a/test/fixtures/onerror-biz/app/controller/home.js b/test/fixtures/onerror-biz/app/controller/home.js new file mode 100644 index 0000000..f24aa10 --- /dev/null +++ b/test/fixtures/onerror-biz/app/controller/home.js @@ -0,0 +1,22 @@ +'use strict'; + +const { Controller } = require('egg'); +const { EggError, EggException } = require('egg-errors'); + + +class HomeController extends Controller { + async throwError() { + throw new Error('error'); + } + + async throwEggError() { + throw new EggError('egg error'); + } + + async throwEggException() { + throw new EggException('egg exception'); + } + +} + +module.exports = HomeController; diff --git a/test/fixtures/onerror-biz/app/router.js b/test/fixtures/onerror-biz/app/router.js new file mode 100644 index 0000000..54a563e --- /dev/null +++ b/test/fixtures/onerror-biz/app/router.js @@ -0,0 +1,7 @@ +'use strict'; + +module.exports = app => { + app.get('/error', app.controller.home.throwError); + app.get('/eggerror', app.controller.home.throwEggError); + app.post('/eggexception', app.controller.home.throwEggException); +}; diff --git a/test/fixtures/onerror-biz/config/config.default.js b/test/fixtures/onerror-biz/config/config.default.js new file mode 100644 index 0000000..e6269cc --- /dev/null +++ b/test/fixtures/onerror-biz/config/config.default.js @@ -0,0 +1,37 @@ +'use strict'; + +exports.logger = { + level: 'NONE', + consoleLevel: 'NONE', +}; + +exports.keys = 'foo,bar'; + +exports.security = { + csrf: false, +}; + +exports.onerror = { + accepts(ctx, ...args) { + console.log(ctx.type, ctx.headers); + const a = ctx.accepts(...args); + console.log(a, ...args); + return a; + }, + application: { + formatJSON(err) { + return { + message: err.message, + code: err.code, + status: err.status, + data: err.data, + }; + }, + formatText(err) { + return err.message; + }, + formatHtml(err) { + return '

' + err.message + '

'; + }, + }, +}; diff --git a/test/fixtures/onerror-biz/package.json b/test/fixtures/onerror-biz/package.json new file mode 100644 index 0000000..2e0400c --- /dev/null +++ b/test/fixtures/onerror-biz/package.json @@ -0,0 +1,3 @@ +{ + "name": "onerror" +} diff --git a/test/onerror.test.js b/test/onerror.test.js index 239d27a..5f4a505 100644 --- a/test/onerror.test.js +++ b/test/onerror.test.js @@ -473,4 +473,28 @@ describe('test/onerror.test.js', () => { }); }); + + + describe.only('biz error', () => { + let app; + before(() => { + app = mm.app({ + baseDir: 'onerror-biz', + }); + return app.ready(); + }); + after(() => app.close()); + + it('should format error', async () => { + await app.httpRequest() + .get('/error') + .set('Accept', 'application/json') + .expect({ + + }) + .expect(200); + }); + + }); + }); From 166e5f2ddf9343fc304e3ba43dc50a2cef4830f5 Mon Sep 17 00:00:00 2001 From: popomore Date: Tue, 25 Dec 2018 01:40:56 +0800 Subject: [PATCH 04/10] f --- app.js | 2 +- app/middleware/egg_onerror_handler.js | 60 ++++++++ app/middleware/onerror_biz_handler.js | 51 ------- config/config.default.js | 24 +++- test/error_handler.test.js | 130 ++++++++++++++++++ .../default-error-handler/app/router.js | 33 +++++ .../config/config.default.js | 9 ++ .../default-error-handler/package.json | 3 + test/onerror.test.js | 23 ---- 9 files changed, 255 insertions(+), 80 deletions(-) create mode 100644 app/middleware/egg_onerror_handler.js delete mode 100644 app/middleware/onerror_biz_handler.js create mode 100644 test/error_handler.test.js create mode 100644 test/fixtures/default-error-handler/app/router.js create mode 100644 test/fixtures/default-error-handler/config/config.default.js create mode 100644 test/fixtures/default-error-handler/package.json diff --git a/app.js b/app.js index 478975a..c2d1b73 100644 --- a/app.js +++ b/app.js @@ -12,7 +12,7 @@ const { } = require('./lib/utils'); module.exports = app => { - app.config.coreMiddleware.push('onerrorBizHandler'); + app.config.coreMiddleware.push('eggOnerrorHandler'); // logging error const config = app.config.onerror; diff --git a/app/middleware/egg_onerror_handler.js b/app/middleware/egg_onerror_handler.js new file mode 100644 index 0000000..6d7e30d --- /dev/null +++ b/app/middleware/egg_onerror_handler.js @@ -0,0 +1,60 @@ +'use strict'; + +const { EggError, ErrorType, InternalServerErrorError } = require('egg-errors'); +const accepts = require('accepts'); + +class UnknownError extends InternalServerErrorError { + constructor(message) { + super(message); + this.message = message; + this.code = 'UNKNOWN_ERROR'; + } +} + +module.exports = (_, app) => { + const errorHandler = app.config.onerror.errorHandler; + const acceptFn = app.config.onerror.accepts || + ((ctx, ...args) => accepts(ctx.req).type(args)); + + return async function onerrorBizHandler(ctx, next) { + try { + await next(); + } catch (e) { + let err = e; + const errorType = EggError.getType(err); + switch (errorType) { + case ErrorType.ERROR: break; + // if error is an EggError Exception, it will pring error log + case ErrorType.EXCEPTION: { + ctx.logger.error(err); + // force set error status + err.status = 500; + break; + } + // if error is not recognized by EggError, + // it will be converted to UnknownError + case ErrorType.BUILTIN: { + err = UnknownError.from(err); + ctx.logger.error(err); + break; + } + // getType not work + default: + throw err; + } + + // handle the error + const contentType = acceptFn(ctx, 'html', 'text', 'json'); + + if (contentType === 'json' && errorHandler.json) { + await errorHandler.json(ctx, err); + } else if (contentType === 'text' && errorHandler.text) { + await errorHandler.text(ctx, err); + } else if (contentType === 'html' && errorHandler.html) { + await errorHandler.html(ctx, err); + } else if (errorHandler.any) { + await errorHandler.any(ctx, err); + } + } + }; +}; diff --git a/app/middleware/onerror_biz_handler.js b/app/middleware/onerror_biz_handler.js deleted file mode 100644 index e67af07..0000000 --- a/app/middleware/onerror_biz_handler.js +++ /dev/null @@ -1,51 +0,0 @@ -'use strict'; - -const { EggError, ErrorType } = require('egg-errors'); -const { accepts } = require('../../lib/utils'); - -module.exports = (_, app) => { - const application = app.config.onerror.application; - - return async function onerrorBizHandler(ctx, next) { - try { - await next(); - } catch (err) { - const fn = app.config.onerror.accepts || accepts; - const contentType = fn(ctx, 'html', 'text', 'json'); - console.info(contentType); - const type = EggError.getType(err); - console.log(type); - switch (type) { - case ErrorType.ERROR: { - ctx.status = err.status || 500; - ctx.body = format(err, contentType); - break; - } - - case ErrorType.EXCEPTION: { - ctx.logger.error(err); - err.status = 500; - ctx.body = format(err, contentType); - break; - } - - case ErrorType.BUILTIN: - default: - throw err; - } - } - - function format(err, contentType) { - if (contentType === 'json' && application.formatJSON) { - ctx.body = application.formatJSON(err); - } else if (contentType === 'text' && application.formatText) { - ctx.body = application.formatText(err); - } else if (contentType === 'html' && application.formatHtml) { - ctx.body = application.formatHtml(err); - } else { - throw err; - } - } - }; - -}; diff --git a/config/config.default.js b/config/config.default.js index f6eabb4..3444087 100644 --- a/config/config.default.js +++ b/config/config.default.js @@ -11,10 +11,24 @@ exports.onerror = { appErrorFilter: null, // default template path templatePath: path.join(__dirname, '../lib/onerror_page.mustache'), - // normalize your error response from error object - application: { - formatJSON: null, - formatText: null, - formatHtml: null, + // handler your error response + errorHandler: { + enable: false, + json: (ctx, err) => { + ctx.status = err.status || 500; + ctx.body = { + code: err.code, + message: err.message, + }; + }, + text: (ctx, err) => { + ctx.status = err.status || 500; + ctx.body = err.message; + }, + html: (ctx, err) => { + ctx.status = err.status || 500; + ctx.body = `

${err.code}

\n
${err.message}
`; + }, + any: null, }, }; diff --git a/test/error_handler.test.js b/test/error_handler.test.js new file mode 100644 index 0000000..013c147 --- /dev/null +++ b/test/error_handler.test.js @@ -0,0 +1,130 @@ +'use strict'; + +const fs = require('fs'); +const pedding = require('pedding'); +const mm = require('egg-mock'); +const rimraf = require('rimraf'); +const path = require('path'); +const assert = require('assert'); + +describe.only('test/error_handler.test.js', () => { + describe.only('default error handler', () => { + let app; + before(() => { + app = mm.app({ + baseDir: 'default-error-handler', + }); + return app.ready(); + }); + after(() => app.close()); + + describe('error', () => { + it('should handler error when accept json', async () => { + await app.httpRequest() + .get('/error') + .set('Accept', 'application/json') + .expect({ + code: 'UNKNOWN_ERROR', + message: 'error', + }) + .expect(500); + }); + + it('should handler error when accept text', async () => { + await app.httpRequest() + .get('/error') + .set('Accept', 'text/plain') + .expect('error') + .expect(500); + }); + + it('should handler error when accept html', async () => { + await app.httpRequest() + .get('/error') + .set('Accept', 'text/html') + .expect('error') + .expect(500); + }); + }); + + describe('egg error', () => { + it('should handler egg error when accept json', async () => { + await app.httpRequest() + .get('/egg-error') + .set('Accept', 'application/json') + .expect({ + code: 'CUSTOM_ERROR', + message: 'egg error', + }) + .expect(422); + }); + + it('should handler egg error when accept text', async () => { + await app.httpRequest() + .get('/egg-error') + .set('Accept', 'text/plain') + .expect('egg error') + .expect(422); + }); + + it('should handler egg error when accept html', async () => { + await app.httpRequest() + .get('/egg-error') + .set('Accept', 'text/html') + .expect('

CUSTOM_ERROR

\n
egg error
') + .expect(422); + }); + }); + + describe('egg exception', () => { + it('should handler egg exception when accept json', async () => { + await app.httpRequest() + .get('/egg-exception') + .set('Accept', 'application/json') + .expect({ + code: 'CUSTOM_EXCEPTION', + message: 'egg exception', + }) + .expect(500); + }); + + it('should handler egg exception when accept text', async () => { + await app.httpRequest() + .get('/egg-exception') + .set('Accept', 'text/plain') + .expect('egg exception') + .expect(500); + }); + + it('should handler egg exception when accept html', async () => { + await app.httpRequest() + .get('/egg-exception') + .set('Accept', 'text/html') + .expect('

CUSTOM_EXCEPTION

\n
egg exception
') + .expect(500); + }); + }); + + }); + + describe('default error handler', () => { + let app; + before(() => { + app = mm.app({ + baseDir: 'custom-error-handler', + }); + return app.ready(); + }); + after(() => app.close()); + + it('should format error', async () => { + await app.httpRequest() + .get('/error') + .set('Accept', 'application/json') + .expect({ + + }) + .expect(200); + }); + }); +}); diff --git a/test/fixtures/default-error-handler/app/router.js b/test/fixtures/default-error-handler/app/router.js new file mode 100644 index 0000000..1a2a6a3 --- /dev/null +++ b/test/fixtures/default-error-handler/app/router.js @@ -0,0 +1,33 @@ +'use strict'; + +const { EggError, EggException } = require('egg-errors'); + +class CustomError extends EggError { + constructor(message) { + super(message); + this.code = 'CUSTOM_ERROR'; + this.status = 422; + } +} + +class CustomException extends EggException { + constructor(message) { + super(message); + this.code = 'CUSTOM_EXCEPTION'; + this.status = 422; + } +} + +module.exports = app => { + app.get('/error', async () => { + throw new Error('error'); + }); + + app.get('/egg-error', async () => { + throw new CustomError('egg error'); + }); + + app.get('/egg-exception', async () => { + throw new CustomException('egg exception'); + }); +}; diff --git a/test/fixtures/default-error-handler/config/config.default.js b/test/fixtures/default-error-handler/config/config.default.js new file mode 100644 index 0000000..3309290 --- /dev/null +++ b/test/fixtures/default-error-handler/config/config.default.js @@ -0,0 +1,9 @@ +'use strict'; + +exports.onerror = { + errorHandler: { + enable: true, + }, +}; + +exports.keys = 'foo,bar'; diff --git a/test/fixtures/default-error-handler/package.json b/test/fixtures/default-error-handler/package.json new file mode 100644 index 0000000..8c431a5 --- /dev/null +++ b/test/fixtures/default-error-handler/package.json @@ -0,0 +1,3 @@ +{ + "name": "egg-onerror" +} diff --git a/test/onerror.test.js b/test/onerror.test.js index 5f4a505..c7f0b87 100644 --- a/test/onerror.test.js +++ b/test/onerror.test.js @@ -474,27 +474,4 @@ describe('test/onerror.test.js', () => { }); - - describe.only('biz error', () => { - let app; - before(() => { - app = mm.app({ - baseDir: 'onerror-biz', - }); - return app.ready(); - }); - after(() => app.close()); - - it('should format error', async () => { - await app.httpRequest() - .get('/error') - .set('Accept', 'application/json') - .expect({ - - }) - .expect(200); - }); - - }); - }); From 11619a796f964fb331d9f444f1db9153baf824f2 Mon Sep 17 00:00:00 2001 From: popomore Date: Thu, 27 Dec 2018 22:42:11 +0800 Subject: [PATCH 05/10] f --- app/middleware/egg_onerror_handler.js | 4 ++-- package.json | 19 ++++++++++--------- test/error_handler.test.js | 12 ++++++------ 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/app/middleware/egg_onerror_handler.js b/app/middleware/egg_onerror_handler.js index 6d7e30d..ba3ab0a 100644 --- a/app/middleware/egg_onerror_handler.js +++ b/app/middleware/egg_onerror_handler.js @@ -1,9 +1,9 @@ 'use strict'; -const { EggError, ErrorType, InternalServerErrorError } = require('egg-errors'); +const { EggError, ErrorType, InternalServerError } = require('egg-errors'); const accepts = require('accepts'); -class UnknownError extends InternalServerErrorError { +class UnknownError extends InternalServerError { constructor(message) { super(message); this.message = message; diff --git a/package.json b/package.json index b8a3cd5..e98dce2 100644 --- a/package.json +++ b/package.json @@ -24,20 +24,21 @@ "onerror" ], "dependencies": { + "accepts": "^1.3.5", "cookie": "^0.3.1", - "egg-errors": "^1.0.1", - "koa-onerror": "^4.0.0", - "mustache": "^2.3.0", + "egg-errors": "^2.1.0", + "koa-onerror": "^4.1.0", + "mustache": "^3.0.1", "stack-trace": "^0.0.10" }, "devDependencies": { - "autod": "^3.0.0", + "autod": "^3.0.1", "egg": "next", - "egg-bin": "^4.3.5", - "egg-ci": "^1.8.0", - "egg-mock": "^3.13.1", - "eslint": "^4.11.0", - "eslint-config-egg": "^5.1.1", + "egg-bin": "^4.9.0", + "egg-ci": "^1.11.0", + "egg-mock": "^3.21.0", + "eslint": "^5.11.1", + "eslint-config-egg": "^7.1.0", "pedding": "^1.1.0", "rimraf": "^2.6.2" }, diff --git a/test/error_handler.test.js b/test/error_handler.test.js index 013c147..0412f51 100644 --- a/test/error_handler.test.js +++ b/test/error_handler.test.js @@ -1,11 +1,11 @@ 'use strict'; -const fs = require('fs'); -const pedding = require('pedding'); +// const fs = require('fs'); +// const pedding = require('pedding'); const mm = require('egg-mock'); -const rimraf = require('rimraf'); -const path = require('path'); -const assert = require('assert'); +// const rimraf = require('rimraf'); +// const path = require('path'); +// const assert = require('assert'); describe.only('test/error_handler.test.js', () => { describe.only('default error handler', () => { @@ -42,7 +42,7 @@ describe.only('test/error_handler.test.js', () => { await app.httpRequest() .get('/error') .set('Accept', 'text/html') - .expect('error') + .expect('

UNKNOWN_ERROR

\n
error
') .expect(500); }); }); From 0ebbae558f8d74dae6396a3b7961f5a978de847a Mon Sep 17 00:00:00 2001 From: popomore Date: Fri, 28 Dec 2018 18:48:44 +0800 Subject: [PATCH 06/10] f --- .gitignore | 1 + app/middleware/egg_onerror_handler.js | 2 + test/error_handler.test.js | 140 ++++++++++++++++-- .../custom-error-handler/app/router.js | 33 +++++ .../config/config.default.js | 25 ++++ .../custom-error-handler/package.json | 3 + 6 files changed, 194 insertions(+), 10 deletions(-) create mode 100644 test/fixtures/custom-error-handler/app/router.js create mode 100644 test/fixtures/custom-error-handler/config/config.default.js create mode 100644 test/fixtures/custom-error-handler/package.json diff --git a/.gitignore b/.gitignore index b4fecda..783b5fa 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ npm-debug.log node_modules/ coverage/ run/ +.nyc_output/ diff --git a/app/middleware/egg_onerror_handler.js b/app/middleware/egg_onerror_handler.js index ba3ab0a..904164c 100644 --- a/app/middleware/egg_onerror_handler.js +++ b/app/middleware/egg_onerror_handler.js @@ -17,6 +17,8 @@ module.exports = (_, app) => { ((ctx, ...args) => accepts(ctx.req).type(args)); return async function onerrorBizHandler(ctx, next) { + if (errorHandler.enable !== true) return next(); + try { await next(); } catch (e) { diff --git a/test/error_handler.test.js b/test/error_handler.test.js index 0412f51..55c7510 100644 --- a/test/error_handler.test.js +++ b/test/error_handler.test.js @@ -7,8 +7,8 @@ const mm = require('egg-mock'); // const path = require('path'); // const assert = require('assert'); -describe.only('test/error_handler.test.js', () => { - describe.only('default error handler', () => { +describe('test/error_handler.test.js', () => { + describe('default error handler', () => { let app; before(() => { app = mm.app({ @@ -45,6 +45,13 @@ describe.only('test/error_handler.test.js', () => { .expect('

UNKNOWN_ERROR

\n
error
') .expect(500); }); + + it('should handler error without accept', async () => { + await app.httpRequest() + .get('/error') + .expect('

UNKNOWN_ERROR

\n
error
') + .expect(500); + }); }); describe('egg error', () => { @@ -74,6 +81,13 @@ describe.only('test/error_handler.test.js', () => { .expect('

CUSTOM_ERROR

\n
egg error
') .expect(422); }); + + it('should handler egg error without accept', async () => { + await app.httpRequest() + .get('/egg-error') + .expect('

CUSTOM_ERROR

\n
egg error
') + .expect(422); + }); }); describe('egg exception', () => { @@ -103,11 +117,18 @@ describe.only('test/error_handler.test.js', () => { .expect('

CUSTOM_EXCEPTION

\n
egg exception
') .expect(500); }); + + it('should handler egg exception without accept', async () => { + await app.httpRequest() + .get('/egg-exception') + .expect('

CUSTOM_EXCEPTION

\n
egg exception
') + .expect(500); + }); }); }); - describe('default error handler', () => { + describe('custom error handler', () => { let app; before(() => { app = mm.app({ @@ -117,14 +138,113 @@ describe.only('test/error_handler.test.js', () => { }); after(() => app.close()); - it('should format error', async () => { - await app.httpRequest() - .get('/error') - .set('Accept', 'application/json') - .expect({ + describe('error', () => { + it('should handler error when accept json', async () => { + await app.httpRequest() + .get('/error') + .set('Accept', 'application/json') + .expect({ + code: 'UNKNOWN_ERROR', + message: 'custom error', + }) + .expect(500); + }); + + it('should handler error when accept text', async () => { + await app.httpRequest() + .get('/error') + .set('Accept', 'text/plain') + .expect('custom error') + .expect(500); + }); + + it('should handler error when accept html', async () => { + await app.httpRequest() + .get('/error') + .set('Accept', 'text/html') + .expect('

UNKNOWN_ERROR

\n
custom error
') + .expect(500); + }); + + it('should handler error without accept', async () => { + await app.httpRequest() + .get('/error') + .expect('

UNKNOWN_ERROR

\n
custom error
') + .expect(500); + }); + }); - }) - .expect(200); + describe('egg error', () => { + it('should handler egg error when accept json', async () => { + await app.httpRequest() + .get('/egg-error') + .set('Accept', 'application/json') + .expect({ + code: 'CUSTOM_ERROR', + message: 'custom egg error', + }) + .expect(422); + }); + + it('should handler egg error when accept text', async () => { + await app.httpRequest() + .get('/egg-error') + .set('Accept', 'text/plain') + .expect('custom egg error') + .expect(422); + }); + + it('should handler egg error when accept html', async () => { + await app.httpRequest() + .get('/egg-error') + .set('Accept', 'text/html') + .expect('

CUSTOM_ERROR

\n
custom egg error
') + .expect(422); + }); + + it('should handler egg error without accept', async () => { + await app.httpRequest() + .get('/egg-error') + .set('Accept', 'text/html') + .expect('

CUSTOM_ERROR

\n
custom egg error
') + .expect(422); + }); + }); + + describe('egg exception', () => { + it('should handler egg exception when accept json', async () => { + await app.httpRequest() + .get('/egg-exception') + .set('Accept', 'application/json') + .expect({ + code: 'CUSTOM_EXCEPTION', + message: 'custom egg exception', + }) + .expect(500); + }); + + it('should handler egg exception when accept text', async () => { + await app.httpRequest() + .get('/egg-exception') + .set('Accept', 'text/plain') + .expect('custom egg exception') + .expect(500); + }); + + it('should handler egg exception when accept html', async () => { + await app.httpRequest() + .get('/egg-exception') + .set('Accept', 'text/html') + .expect('

CUSTOM_EXCEPTION

\n
custom egg exception
') + .expect(500); + }); + + it('should handler egg exception without accept', async () => { + await app.httpRequest() + .get('/egg-exception') + .expect('

CUSTOM_EXCEPTION

\n
custom egg exception
') + .expect(500); + }); }); }); }); diff --git a/test/fixtures/custom-error-handler/app/router.js b/test/fixtures/custom-error-handler/app/router.js new file mode 100644 index 0000000..1a2a6a3 --- /dev/null +++ b/test/fixtures/custom-error-handler/app/router.js @@ -0,0 +1,33 @@ +'use strict'; + +const { EggError, EggException } = require('egg-errors'); + +class CustomError extends EggError { + constructor(message) { + super(message); + this.code = 'CUSTOM_ERROR'; + this.status = 422; + } +} + +class CustomException extends EggException { + constructor(message) { + super(message); + this.code = 'CUSTOM_EXCEPTION'; + this.status = 422; + } +} + +module.exports = app => { + app.get('/error', async () => { + throw new Error('error'); + }); + + app.get('/egg-error', async () => { + throw new CustomError('egg error'); + }); + + app.get('/egg-exception', async () => { + throw new CustomException('egg exception'); + }); +}; diff --git a/test/fixtures/custom-error-handler/config/config.default.js b/test/fixtures/custom-error-handler/config/config.default.js new file mode 100644 index 0000000..85d39ee --- /dev/null +++ b/test/fixtures/custom-error-handler/config/config.default.js @@ -0,0 +1,25 @@ +'use strict'; + +exports.onerror = { + errorHandler: { + enable: true, + json: (ctx, err) => { + ctx.status = err.status || 500; + ctx.body = { + code: err.code, + message: 'custom ' + err.message, + }; + }, + text: (ctx, err) => { + ctx.status = err.status || 500; + ctx.body = 'custom ' + err.message; + }, + html: (ctx, err) => { + ctx.status = err.status || 500; + ctx.body = `

${err.code}

\n
custom ${err.message}
`; + }, + any: null, + }, +}; + +exports.keys = 'foo,bar'; diff --git a/test/fixtures/custom-error-handler/package.json b/test/fixtures/custom-error-handler/package.json new file mode 100644 index 0000000..8c431a5 --- /dev/null +++ b/test/fixtures/custom-error-handler/package.json @@ -0,0 +1,3 @@ +{ + "name": "egg-onerror" +} From 4c6b87a455c862085d6887fd957a79acdf186645 Mon Sep 17 00:00:00 2001 From: popomore Date: Fri, 28 Dec 2018 21:12:16 +0800 Subject: [PATCH 07/10] f --- app/middleware/egg_onerror_handler.js | 12 ++- config/config.default.js | 18 +---- test/error_handler.test.js | 76 +++++++++++-------- .../config/config.default.js | 7 ++ 4 files changed, 61 insertions(+), 52 deletions(-) diff --git a/app/middleware/egg_onerror_handler.js b/app/middleware/egg_onerror_handler.js index 904164c..2d16ccd 100644 --- a/app/middleware/egg_onerror_handler.js +++ b/app/middleware/egg_onerror_handler.js @@ -13,10 +13,10 @@ class UnknownError extends InternalServerError { module.exports = (_, app) => { const errorHandler = app.config.onerror.errorHandler; - const acceptFn = app.config.onerror.accepts || + const acceptContentType = app.config.onerror.accepts || ((ctx, ...args) => accepts(ctx.req).type(args)); - return async function onerrorBizHandler(ctx, next) { + return async function bizErrorHandler(ctx, next) { if (errorHandler.enable !== true) return next(); try { @@ -26,6 +26,7 @@ module.exports = (_, app) => { const errorType = EggError.getType(err); switch (errorType) { case ErrorType.ERROR: break; + // if error is an EggError Exception, it will pring error log case ErrorType.EXCEPTION: { ctx.logger.error(err); @@ -33,6 +34,7 @@ module.exports = (_, app) => { err.status = 500; break; } + // if error is not recognized by EggError, // it will be converted to UnknownError case ErrorType.BUILTIN: { @@ -40,13 +42,13 @@ module.exports = (_, app) => { ctx.logger.error(err); break; } + // getType not work default: - throw err; } // handle the error - const contentType = acceptFn(ctx, 'html', 'text', 'json'); + const contentType = acceptContentType(ctx, 'html', 'text', 'json'); if (contentType === 'json' && errorHandler.json) { await errorHandler.json(ctx, err); @@ -56,6 +58,8 @@ module.exports = (_, app) => { await errorHandler.html(ctx, err); } else if (errorHandler.any) { await errorHandler.any(ctx, err); + } else { + throw err; } } }; diff --git a/config/config.default.js b/config/config.default.js index 3444087..04cd2ff 100644 --- a/config/config.default.js +++ b/config/config.default.js @@ -14,21 +14,9 @@ exports.onerror = { // handler your error response errorHandler: { enable: false, - json: (ctx, err) => { - ctx.status = err.status || 500; - ctx.body = { - code: err.code, - message: err.message, - }; - }, - text: (ctx, err) => { - ctx.status = err.status || 500; - ctx.body = err.message; - }, - html: (ctx, err) => { - ctx.status = err.status || 500; - ctx.body = `

${err.code}

\n
${err.message}
`; - }, + json: null, + text: null, + html: null, any: null, }, }; diff --git a/test/error_handler.test.js b/test/error_handler.test.js index 55c7510..7df84ae 100644 --- a/test/error_handler.test.js +++ b/test/error_handler.test.js @@ -19,109 +19,119 @@ describe('test/error_handler.test.js', () => { after(() => app.close()); describe('error', () => { - it('should handler error when accept json', async () => { + it('should handler error', async () => { await app.httpRequest() .get('/error') .set('Accept', 'application/json') .expect({ code: 'UNKNOWN_ERROR', - message: 'error', + message: 'any error', }) .expect(500); - }); - it('should handler error when accept text', async () => { await app.httpRequest() .get('/error') .set('Accept', 'text/plain') - .expect('error') + .expect({ + code: 'UNKNOWN_ERROR', + message: 'any error', + }) .expect(500); - }); - it('should handler error when accept html', async () => { await app.httpRequest() .get('/error') .set('Accept', 'text/html') - .expect('

UNKNOWN_ERROR

\n
error
') + .expect({ + code: 'UNKNOWN_ERROR', + message: 'any error', + }) .expect(500); - }); - it('should handler error without accept', async () => { await app.httpRequest() .get('/error') - .expect('

UNKNOWN_ERROR

\n
error
') + .expect({ + code: 'UNKNOWN_ERROR', + message: 'any error', + }) .expect(500); }); + }); describe('egg error', () => { - it('should handler egg error when accept json', async () => { + it('should handler egg error', async () => { await app.httpRequest() .get('/egg-error') .set('Accept', 'application/json') .expect({ code: 'CUSTOM_ERROR', - message: 'egg error', + message: 'any egg error', }) .expect(422); - }); - it('should handler egg error when accept text', async () => { await app.httpRequest() .get('/egg-error') .set('Accept', 'text/plain') - .expect('egg error') + .expect({ + code: 'CUSTOM_ERROR', + message: 'any egg error', + }) .expect(422); - }); - it('should handler egg error when accept html', async () => { await app.httpRequest() .get('/egg-error') .set('Accept', 'text/html') - .expect('

CUSTOM_ERROR

\n
egg error
') + .expect({ + code: 'CUSTOM_ERROR', + message: 'any egg error', + }) .expect(422); - }); - it('should handler egg error without accept', async () => { await app.httpRequest() .get('/egg-error') - .expect('

CUSTOM_ERROR

\n
egg error
') + .expect({ + code: 'CUSTOM_ERROR', + message: 'any egg error', + }) .expect(422); }); }); describe('egg exception', () => { - it('should handler egg exception when accept json', async () => { + it('should handler egg exception', async () => { await app.httpRequest() .get('/egg-exception') .set('Accept', 'application/json') .expect({ code: 'CUSTOM_EXCEPTION', - message: 'egg exception', + message: 'any egg exception', }) .expect(500); - }); - it('should handler egg exception when accept text', async () => { await app.httpRequest() .get('/egg-exception') .set('Accept', 'text/plain') - .expect('egg exception') + .expect({ + code: 'CUSTOM_EXCEPTION', + message: 'any egg exception', + }) .expect(500); - }); - it('should handler egg exception when accept html', async () => { await app.httpRequest() .get('/egg-exception') .set('Accept', 'text/html') - .expect('

CUSTOM_EXCEPTION

\n
egg exception
') + .expect({ + code: 'CUSTOM_EXCEPTION', + message: 'any egg exception', + }) .expect(500); - }); - it('should handler egg exception without accept', async () => { await app.httpRequest() .get('/egg-exception') - .expect('

CUSTOM_EXCEPTION

\n
egg exception
') + .expect({ + code: 'CUSTOM_EXCEPTION', + message: 'any egg exception', + }) .expect(500); }); }); diff --git a/test/fixtures/default-error-handler/config/config.default.js b/test/fixtures/default-error-handler/config/config.default.js index 3309290..621b74d 100644 --- a/test/fixtures/default-error-handler/config/config.default.js +++ b/test/fixtures/default-error-handler/config/config.default.js @@ -3,6 +3,13 @@ exports.onerror = { errorHandler: { enable: true, + any: (ctx, err) => { + ctx.status = err.status; + ctx.body = { + code: err.code, + message: 'any ' + err.message, + }; + }, }, }; From 3bb19e61364f61eecf964852195a5bced8654ee2 Mon Sep 17 00:00:00 2001 From: popomore Date: Fri, 28 Dec 2018 21:16:56 +0800 Subject: [PATCH 08/10] f --- agent.js | 15 ++-- app.js | 233 ++++++++++++++++++++++++++++--------------------------- 2 files changed, 130 insertions(+), 118 deletions(-) diff --git a/agent.js b/agent.js index 99b34c2..d6c4988 100644 --- a/agent.js +++ b/agent.js @@ -1,8 +1,13 @@ 'use strict'; -module.exports = agent => { - // should watch error event - agent.on('error', err => { - agent.coreLogger.error(err); - }); +module.exports = class { + constructor(agent) { + this.agent = agent; + } + configDidLoad() { + // should watch error event + this.agent.on('error', err => { + this.agent.coreLogger.error(err); + }); + } }; diff --git a/app.js b/app.js index c2d1b73..b9666d4 100644 --- a/app.js +++ b/app.js @@ -11,142 +11,149 @@ const { accepts, } = require('./lib/utils'); -module.exports = app => { - app.config.coreMiddleware.push('eggOnerrorHandler'); +module.exports = class { + constructor(app) { + this.app = app; + } + configDidLoad() { + const app = this.app; - // logging error - const config = app.config.onerror; - const viewTemplate = fs.readFileSync(config.templatePath, 'utf8'); + app.config.coreMiddleware.push('eggOnerrorHandler'); - app.on('error', (err, ctx) => { - ctx = ctx || app.createAnonymousContext(); - if (config.appErrorFilter && !config.appErrorFilter(err, ctx)) return; + // logging error + const config = app.config.onerror; + const viewTemplate = fs.readFileSync(config.templatePath, 'utf8'); - const status = detectStatus(err); - // 5xx - if (status >= 500) { + app.on('error', (err, ctx) => { + ctx = ctx || app.createAnonymousContext(); + if (config.appErrorFilter && !config.appErrorFilter(err, ctx)) return; + + const status = detectStatus(err); + // 5xx + if (status >= 500) { + try { + ctx.logger.error(err); + } catch (ex) { + app.logger.error(err); + app.logger.error(ex); + } + return; + } + + // 4xx try { - ctx.logger.error(err); + ctx.logger.warn(err); } catch (ex) { - app.logger.error(err); + app.logger.warn(err); app.logger.error(ex); } - return; - } - - // 4xx - try { - ctx.logger.warn(err); - } catch (ex) { - app.logger.warn(err); - app.logger.error(ex); - } - }); - - const errorOptions = { - // support customize accepts function - accepts(...args) { - const fn = config.accepts || accepts; - return fn(this, ...args); - }, - - html(err) { - const status = detectStatus(err); - const errorPageUrl = typeof config.errorPageUrl === 'function' - ? config.errorPageUrl(err, this) - : config.errorPageUrl; - - // keep the real response status - this.realStatus = status; - // don't respond any error message in production env - if (isProd(app)) { - // 5xx - if (status >= 500) { - if (errorPageUrl) { - const statusQuery = - (errorPageUrl.indexOf('?') > 0 ? '&' : '?') + - `real_status=${status}`; - return this.redirect(errorPageUrl + statusQuery); + }); + + const errorOptions = { + // support customize accepts function + accepts(...args) { + const fn = config.accepts || accepts; + return fn(this, ...args); + }, + + html(err) { + const status = detectStatus(err); + const errorPageUrl = typeof config.errorPageUrl === 'function' + ? config.errorPageUrl(err, this) + : config.errorPageUrl; + + // keep the real response status + this.realStatus = status; + // don't respond any error message in production env + if (isProd(app)) { + // 5xx + if (status >= 500) { + if (errorPageUrl) { + const statusQuery = + (errorPageUrl.indexOf('?') > 0 ? '&' : '?') + + `real_status=${status}`; + return this.redirect(errorPageUrl + statusQuery); + } + this.status = 500; + this.body = `

Internal Server Error, real status: ${status}

`; + return; } - this.status = 500; - this.body = `

Internal Server Error, real status: ${status}

`; + // 4xx + this.status = status; + this.body = `

${status} ${http.STATUS_CODES[status]}

`; + return; + } + // show simple error format for unittest + if (app.config.env === 'unittest') { + this.status = status; + this.body = `${err.name}: ${err.message}\n${err.stack}`; return; } - // 4xx - this.status = status; - this.body = `

${status} ${http.STATUS_CODES[status]}

`; - return; - } - // show simple error format for unittest - if (app.config.env === 'unittest') { - this.status = status; - this.body = `${err.name}: ${err.message}\n${err.stack}`; - return; - } - - const errorView = new ErrorView(this, err, viewTemplate); - this.body = errorView.toHTML(); - }, - json(err) { - const status = detectStatus(err); - let errorJson = {}; + const errorView = new ErrorView(this, err, viewTemplate); + this.body = errorView.toHTML(); + }, - this.status = status; - const code = err.code || err.type; - const message = detectErrorMessage(this, err); + json(err) { + const status = detectStatus(err); + let errorJson = {}; - if (isProd(app)) { - // 5xx server side error - if (status >= 500) { - errorJson = { - code, - // don't respond any error message in production env - message: http.STATUS_CODES[status], - }; + this.status = status; + const code = err.code || err.type; + const message = detectErrorMessage(this, err); + + if (isProd(app)) { + // 5xx server side error + if (status >= 500) { + errorJson = { + code, + // don't respond any error message in production env + message: http.STATUS_CODES[status], + }; + } else { + // 4xx client side error + // addition `errors` + errorJson = { + code, + message, + errors: err.errors, + }; + } } else { - // 4xx client side error - // addition `errors` errorJson = { code, message, errors: err.errors, }; - } - } else { - errorJson = { - code, - message, - errors: err.errors, - }; - - if (status >= 500) { - // provide detail error stack in local env - errorJson.stack = err.stack; - errorJson.name = err.name; - for (const key in err) { - if (!errorJson[key]) { - errorJson[key] = err[key]; + + if (status >= 500) { + // provide detail error stack in local env + errorJson.stack = err.stack; + errorJson.name = err.name; + for (const key in err) { + if (!errorJson[key]) { + errorJson[key] = err[key]; + } } } } - } - this.body = errorJson; - }, + this.body = errorJson; + }, - js(err) { - errorOptions.json.call(this, err, this); + js(err) { + errorOptions.json.call(this, err, this); - if (this.createJsonpBody) { - this.createJsonpBody(this.body); - } - }, - }; - - // support customize error response - [ 'all', 'html', 'json', 'text', 'js' ].forEach(type => { - if (config[type]) errorOptions[type] = config[type]; - }); - onerror(app, errorOptions); + if (this.createJsonpBody) { + this.createJsonpBody(this.body); + } + }, + }; + + // support customize error response + [ 'all', 'html', 'json', 'text', 'js' ].forEach(type => { + if (config[type]) errorOptions[type] = config[type]; + }); + onerror(app, errorOptions); + } }; From d9ab5598ca05856addd029a7479376d39690b8a2 Mon Sep 17 00:00:00 2001 From: popomore Date: Sun, 30 Dec 2018 01:05:36 +0800 Subject: [PATCH 09/10] f --- config/config.default.js | 2 + test/error_handler.test.js | 38 ++++++++++++++++--- .../app/router.js | 0 .../config/config.default.js | 0 .../package.json | 0 .../unmatch-error-handler/app/router.js | 7 ++++ .../config/config.default.js | 9 +++++ .../unmatch-error-handler/package.json | 3 ++ 8 files changed, 53 insertions(+), 6 deletions(-) rename test/fixtures/{default-error-handler => any-error-handler}/app/router.js (100%) rename test/fixtures/{default-error-handler => any-error-handler}/config/config.default.js (100%) rename test/fixtures/{default-error-handler => any-error-handler}/package.json (100%) create mode 100644 test/fixtures/unmatch-error-handler/app/router.js create mode 100644 test/fixtures/unmatch-error-handler/config/config.default.js create mode 100644 test/fixtures/unmatch-error-handler/package.json diff --git a/config/config.default.js b/config/config.default.js index 04cd2ff..2ea0a89 100644 --- a/config/config.default.js +++ b/config/config.default.js @@ -2,6 +2,8 @@ const path = require('path'); exports.onerror = { + // function that can customize which content type will be accept + accepts: null, // 5xx error will redirect to ${errorPageUrl} // won't redirect in local env errorPageUrl: '', diff --git a/test/error_handler.test.js b/test/error_handler.test.js index 7df84ae..bc48432 100644 --- a/test/error_handler.test.js +++ b/test/error_handler.test.js @@ -1,18 +1,14 @@ 'use strict'; -// const fs = require('fs'); -// const pedding = require('pedding'); const mm = require('egg-mock'); -// const rimraf = require('rimraf'); -// const path = require('path'); -// const assert = require('assert'); +const assert = require('assert'); describe('test/error_handler.test.js', () => { describe('default error handler', () => { let app; before(() => { app = mm.app({ - baseDir: 'default-error-handler', + baseDir: 'any-error-handler', }); return app.ready(); }); @@ -257,4 +253,34 @@ describe('test/error_handler.test.js', () => { }); }); }); + + describe('unmatch error handler', () => { + let app; + before(() => { + app = mm.app({ + baseDir: 'unmatch-error-handler', + }); + return app.ready(); + }); + after(() => app.close()); + + it('should throw when accept json', async () => { + const res = await app.httpRequest() + .get('/error') + .set('Accept', 'application/json') + .expect(500); + assert(res.body.code === 'UNKNOWN_ERROR'); + assert(res.body.message === 'error'); + }); + + it('should throw when accept html', async () => { + const res = await app.httpRequest() + .get('/error') + .set('Accept', 'text/html') + .expect(500); + + assert(res.text.includes('UnknownError: error')); + }); + }); + }); diff --git a/test/fixtures/default-error-handler/app/router.js b/test/fixtures/any-error-handler/app/router.js similarity index 100% rename from test/fixtures/default-error-handler/app/router.js rename to test/fixtures/any-error-handler/app/router.js diff --git a/test/fixtures/default-error-handler/config/config.default.js b/test/fixtures/any-error-handler/config/config.default.js similarity index 100% rename from test/fixtures/default-error-handler/config/config.default.js rename to test/fixtures/any-error-handler/config/config.default.js diff --git a/test/fixtures/default-error-handler/package.json b/test/fixtures/any-error-handler/package.json similarity index 100% rename from test/fixtures/default-error-handler/package.json rename to test/fixtures/any-error-handler/package.json diff --git a/test/fixtures/unmatch-error-handler/app/router.js b/test/fixtures/unmatch-error-handler/app/router.js new file mode 100644 index 0000000..3b36cd4 --- /dev/null +++ b/test/fixtures/unmatch-error-handler/app/router.js @@ -0,0 +1,7 @@ +'use strict'; + +module.exports = app => { + app.get('/error', async () => { + throw new Error('error'); + }); +}; diff --git a/test/fixtures/unmatch-error-handler/config/config.default.js b/test/fixtures/unmatch-error-handler/config/config.default.js new file mode 100644 index 0000000..3309290 --- /dev/null +++ b/test/fixtures/unmatch-error-handler/config/config.default.js @@ -0,0 +1,9 @@ +'use strict'; + +exports.onerror = { + errorHandler: { + enable: true, + }, +}; + +exports.keys = 'foo,bar'; diff --git a/test/fixtures/unmatch-error-handler/package.json b/test/fixtures/unmatch-error-handler/package.json new file mode 100644 index 0000000..8c431a5 --- /dev/null +++ b/test/fixtures/unmatch-error-handler/package.json @@ -0,0 +1,3 @@ +{ + "name": "egg-onerror" +} From 8e18cfb1c18e2263158c6bd8460a0defb4e61de8 Mon Sep 17 00:00:00 2001 From: popomore Date: Sun, 30 Dec 2018 01:37:56 +0800 Subject: [PATCH 10/10] f --- app.js | 10 ++++++---- app/middleware/egg_onerror_handler.js | 2 -- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app.js b/app.js index b9666d4..310509d 100644 --- a/app.js +++ b/app.js @@ -18,12 +18,14 @@ module.exports = class { configDidLoad() { const app = this.app; - app.config.coreMiddleware.push('eggOnerrorHandler'); - - // logging error const config = app.config.onerror; const viewTemplate = fs.readFileSync(config.templatePath, 'utf8'); + // push biz error middleware + if (config.errorHandler.enable === true) { + app.config.coreMiddleware.push('eggOnerrorHandler'); + } + app.on('error', (err, ctx) => { ctx = ctx || app.createAnonymousContext(); if (config.appErrorFilter && !config.appErrorFilter(err, ctx)) return; @@ -70,7 +72,7 @@ module.exports = class { if (status >= 500) { if (errorPageUrl) { const statusQuery = - (errorPageUrl.indexOf('?') > 0 ? '&' : '?') + + (errorPageUrl.includes('?') ? '&' : '?') + `real_status=${status}`; return this.redirect(errorPageUrl + statusQuery); } diff --git a/app/middleware/egg_onerror_handler.js b/app/middleware/egg_onerror_handler.js index 2d16ccd..03c54eb 100644 --- a/app/middleware/egg_onerror_handler.js +++ b/app/middleware/egg_onerror_handler.js @@ -17,8 +17,6 @@ module.exports = (_, app) => { ((ctx, ...args) => accepts(ctx.req).type(args)); return async function bizErrorHandler(ctx, next) { - if (errorHandler.enable !== true) return next(); - try { await next(); } catch (e) {