-
Notifications
You must be signed in to change notification settings - Fork 14
feat: add biz handler #29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 4 commits
57916a5
f0cd1b4
7169690
166e5f2
11619a7
0ebbae5
4c6b87a
3bb19e6
d9ab559
8e18cfb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 || | ||
popomore marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
((ctx, ...args) => accepts(ctx.req).type(args)); | ||
|
||
return async function onerrorBizHandler(ctx, next) { | ||
popomore marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
try { | ||
await next(); | ||
} catch (e) { | ||
let err = e; | ||
const errorType = EggError.getType(err); | ||
switch (errorType) { | ||
case ErrorType.ERROR: break; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 又看了一次代码,还是很难理解,日志什么时候该打,什么时候该使用 EggError,什么时候用 EXCEPTION 。。。 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Error 是用于终端的,比如检验错误,验权,不需要打 error。exception 是系统错误,常见的 500,或者底层抛的错,需要打 error 日志。针对响应则用 errorHandler 统一处理。 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 先整理个文章出来吧,这种经验实践类的功能看代码老是跟不上思路。。。 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 我理解的是 ERROR 是类似业务处理报错,是业务上预期的错误,EXCEPTION 是异常,非预期的程序错误,所以需要打印日志。 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 如果是终端的,那么应该使用 ClientError 会更好,不用思考就明白了。 |
||
// 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); | ||
} | ||
} | ||
}; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,4 +11,24 @@ exports.onerror = { | |
appErrorFilter: null, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 上面的 |
||
// default template path | ||
templatePath: path.join(__dirname, '../lib/onerror_page.mustache'), | ||
// handler your error response | ||
errorHandler: { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 这里重新实现了一下 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 感觉这个应该是一个配置,而是约定在 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 我感觉这里比较简单,写配置比较容易想到,约定文件太多也不是非常好。配置还有一个好处是有 dump。 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. egg 的文档将使用说明写起来看看没问题就可以合并发布了。 |
||
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 = `<h2>${err.code}</h2>\n<div>${err.message}</div>`; | ||
}, | ||
any: null, | ||
}, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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('<h2>CUSTOM_ERROR</h2>\n<div>egg error</div>') | ||
.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('<h2>CUSTOM_EXCEPTION</h2>\n<div>egg exception</div>') | ||
.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); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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'); | ||
}); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
'use strict'; | ||
|
||
exports.onerror = { | ||
errorHandler: { | ||
enable: true, | ||
}, | ||
}; | ||
|
||
exports.keys = 'foo,bar'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
{ | ||
"name": "egg-onerror" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 这个还是不要改原生了?就作为 BUILDIN 错误好了 |
||
} | ||
|
||
async throwEggError() { | ||
throw new EggError('egg error'); | ||
} | ||
|
||
async throwEggException() { | ||
throw new EggException('egg exception'); | ||
} | ||
|
||
} | ||
|
||
module.exports = HomeController; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 '<h2>' + err.message + '</h2>'; | ||
}, | ||
}, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
{ | ||
"name": "onerror" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -473,4 +473,5 @@ describe('test/onerror.test.js', () => { | |
}); | ||
|
||
}); | ||
|
||
}); |
Uh oh!
There was an error while loading. Please reload this page.