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
Make exception handling more robust for background updates and release v0.7.3 #117
Make exception handling more robust for background updates and release v0.7.3 #117
Conversation
return P.try(function() { | ||
return P.each(res.items.reverse(), handler.handleRow.bind(handler)); | ||
}).catch(function() { | ||
// noop here for now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could add logging here, at least we would know smth happened.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
LGTM, except one suggestion about logging. |
I have released v0.7.2 in order to be able to re-deploy the last version of RESTBase with only the new version of the storage module. |
v0.7.3 has been released to npm, https://gerrit.wikimedia.org/r/223297 is the deployment candidate only bumping the version |
}) | ||
.catch(function(err) { | ||
if (!options.retries) { | ||
throw err; | ||
return P.reject(err); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is equivalent to throw, as it is inside a promise handler.
LGTM overall, just two small cases where I think a .catch is not needed / doesn't make a difference. |
@gwicke forgot to remove these useless (and desperate) changes. Removed them. |
Make exception handling more robust for background updates and release v0.7.3
Bug: T104581