Hapi v2 rebased #537
Hapi v2 rebased #537
Conversation
Conflicts: routes/defaults.js
Conflicts: routes/defaults.js
|
@rfk can you take a look at this? I recommend excluding the last 2 commit when you diff, they're cosmetic and screw it all up. Specifically I'd like your feedback on |
| @@ -122,7 +138,8 @@ module.exports = function (path, url, Hapi, toobusy) { | |||
| if (toobusy()) { | |||
| exit = error.serviceUnavailable() | |||
| } | |||
| log.begin('server.onRequest', request); | |||
| log.begin('--- ' + request.id + ' ---', request); | |||
| log.begin('=== server.onRequest ===', request); | |||
There was a problem hiding this comment.
we don't want multiple log.begins and no "---" or "===" funny stuff
|
Just to ack your request here, I'm burried under some syncserver stuff at the moment but should be able to look at this later today or tomorrow |
|
@rfk no worries, at your leisure 🍸 |
| var object = Hoek.applyToDefaults(DEFAULTS, srcObject) | ||
| if (srcObject.hasOwnProperty('message')) { | ||
| object.message = srcObject.message | ||
| AppError.translate = function (payload) { |
There was a problem hiding this comment.
I think this is non-obvious enough to deserve a brief comment line e.g. "translate from internal hapi error objects into our standardized format"
|
This looks good to me. There's no obviously "right" solution to our error-management needs, and I think what you've done here appropriately minimises the ickyness. It may bite us if internal details of hapi happen to change, but hey, that's why we have tests! |
|
Thanks @rfk, I've got a branch in progress with more changes to |
Fixes #495.