properly handle promise exceptions on startup #78
Conversation
|
The test fails on node 6 because it doesn't install |
|
I didn't look through the internals of the |
app.js
Outdated
| process.env.NO_PROXY = app.conf.no_proxy_list.join(','); | ||
| } else { | ||
| process.env.NO_PROXY = app.conf.no_proxy_list; | ||
| return BBPromise.try(() => { |
There was a problem hiding this comment.
To avoid a gigantic diff that will not be easy to merge into the services we can leave the initApp function internals untouched and just do const initApp = BBPromise.method(() => ... in the function initialization. That one-liner will have the same effect as this huge diff.
There was a problem hiding this comment.
I updated it to be exactly as before, but the indent needs to be increased by 4 spaces, or else the eslint will complain
There was a problem hiding this comment.
Ye, that's why I'm suggesting the BBPromise.method feature - then you won't need to update the indentation.
app.js
Outdated
| app.use('/static', express.static(__dirname + '/static')); | ||
| return app; | ||
| }).then(createServer); | ||
| .then(loadRoutes) |
There was a problem hiding this comment.
We use the code-style where the .then starts on the new line without indentation.
nyurik
force-pushed
the
appclean
branch
2 times, most recently
from
3b9fc58 to
8cec918
Compare
|
@Pchelolo good catch, I didn't know of BBPromise.method(). Fixed. |
|
LGTM, but not sure why changes from my previous PR are showing up in here, but it should be fine to merge. @d00rman ? |
|
@Pchelolo because i rebased my changes on top of yours :) |
|
LGTM, but (a) needs a rebase; and (b) please do not remove the comment separator lines in function headers. |
3 participants
cleaned up app.js to fix how promises are created/processed.
The old code would throw an exception outside of the promise chain in case of an error in config.
Also, auto-cleaned up the code (spacing, newlines, curly braces)