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

feat: Handle CronJob not found #43

Merged
merged 2 commits into from
Jul 13, 2024
Merged

feat: Handle CronJob not found #43

merged 2 commits into from
Jul 13, 2024

Conversation

meyfa
Copy link
Member

@meyfa meyfa commented Jul 13, 2024

Before this patch, if the Renovate CronJob could not be found, we would return a 500 Internal Server Error and the UI would look broken. Now, we return a 404 Not Found, log a warning, and let the frontend show an error message component. The info cards are updated to properly handle this case.

Screenshot:

image

@meyfa meyfa requested a review from a team as a code owner July 13, 2024 14:34
Before this patch, if the Renovate CronJob could not be found, we
would return a 500 Internal Server Error and the UI would look
broken. Now, we return a 404 Not Found, log a warning, and let the
frontend show an error message component. The info cards are
updated to properly handle this case.
@meyfa meyfa force-pushed the feat/cronjob-404 branch from 9794dde to fdffa60 Compare July 13, 2024 14:35
@meyfa
Copy link
Member Author

meyfa commented Jul 13, 2024

As a side-effect, since literally every other controller also invokes getCronJob() internally, the other API routes no longer return 500 Internal Server Error either, but instead return 404 when the CronJob is missing.

@meyfa meyfa merged commit b8a094b into main Jul 13, 2024
3 checks passed
@meyfa meyfa deleted the feat/cronjob-404 branch July 13, 2024 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants