-
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 8 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 |
---|---|---|
|
@@ -3,3 +3,4 @@ npm-debug.log | |
node_modules/ | ||
coverage/ | ||
run/ | ||
.nyc_output/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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); | ||
}); | ||
} | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,140 +11,149 @@ const { | |
accepts, | ||
} = require('./lib/utils'); | ||
|
||
module.exports = app => { | ||
// logging error | ||
const config = app.config.onerror; | ||
const viewTemplate = fs.readFileSync(config.templatePath, 'utf8'); | ||
|
||
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) { | ||
module.exports = class { | ||
constructor(app) { | ||
this.app = app; | ||
} | ||
configDidLoad() { | ||
const app = this.app; | ||
|
||
app.config.coreMiddleware.push('eggOnerrorHandler'); | ||
|
||
// logging error | ||
const config = app.config.onerror; | ||
const viewTemplate = fs.readFileSync(config.templatePath, 'utf8'); | ||
|
||
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() { | ||
const fn = config.accepts || accepts; | ||
return fn(this); | ||
}, | ||
|
||
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 = `<h2>Internal Server Error, real status: ${status}</h2>`; | ||
return; | ||
} | ||
this.status = 500; | ||
this.body = `<h2>Internal Server Error, real status: ${status}</h2>`; | ||
// 4xx | ||
this.status = status; | ||
this.body = `<h2>${status} ${http.STATUS_CODES[status]}</h2>`; | ||
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 = `<h2>${status} ${http.STATUS_CODES[status]}</h2>`; | ||
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); | ||
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. 这里是不是可以再包一层,这样的话,在 custom error handler 里面,可以支持 |
||
} | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
'use strict'; | ||
|
||
const { EggError, ErrorType, InternalServerError } = require('egg-errors'); | ||
const accepts = require('accepts'); | ||
|
||
class UnknownError extends InternalServerError { | ||
constructor(message) { | ||
super(message); | ||
this.message = message; | ||
this.code = 'UNKNOWN_ERROR'; | ||
} | ||
} | ||
|
||
module.exports = (_, app) => { | ||
const errorHandler = app.config.onerror.errorHandler; | ||
const acceptContentType = app.config.onerror.accepts || | ||
((ctx, ...args) => accepts(ctx.req).type(args)); | ||
|
||
return async function bizErrorHandler(ctx, next) { | ||
if (errorHandler.enable !== true) return 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: | ||
} | ||
|
||
// handle the error | ||
const contentType = acceptContentType(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); | ||
} else { | ||
throw err; | ||
} | ||
} | ||
}; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,4 +11,12 @@ 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: null, | ||
text: null, | ||
html: null, | ||
any: null, | ||
}, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,19 +24,21 @@ | |
"onerror" | ||
], | ||
"dependencies": { | ||
"accepts": "^1.3.5", | ||
"cookie": "^0.3.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", | ||
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-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" | ||
}, | ||
|
Uh oh!
There was an error while loading. Please reload this page.