Skip to content
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

add error rescue for cron start method #263

Merged
merged 2 commits into from
May 24, 2016
Merged

add error rescue for cron start method #263

merged 2 commits into from
May 24, 2016

Conversation

carlad
Copy link
Contributor

@carlad carlad commented May 24, 2016

There's an issue where any error in the start method is not rescued, and will bubble up to the start_all method, causing all subsequent crons to be aborted.

This change has been deployed to staging and everything seems to be working.

@joecorcoran
Copy link
Contributor

Looks good. Only thing I would add is that it would be more useful to log the error with whatever logger we usually use in this app, rather than using puts.

@carlad carlad merged commit 6e51368 into master May 24, 2016
@renee-travisci renee-travisci deleted the cd-cron-error branch May 24, 2016 15:24
@rkh
Copy link
Contributor

rkh commented May 24, 2016

Looks good. Only thing I would add is that it would be more useful to log the error with whatever logger we usually use in this app, rather than using puts.

I think that's accessible via Travis.logger. Also, we might want to track exceptions in Sentry as well, so they don't get lost.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants