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

Fix case where empty HTTP response caused panic in reconcile loop #98

Merged
merged 6 commits into from
Apr 7, 2021

Conversation

coro
Copy link
Contributor

@coro coro commented Apr 7, 2021

This closes #88

Note to reviewers: remember to look at the commits in this PR and consider if they can be squashed

Summary Of Changes

Both validateResponse and validateResponseForDeletion can panic if they are asked to validate a nil-reponse object. Rabbithole returns this in some cases, where there is a Go error or a 401 from the API.

For both of these, the function now returns an error if both err and the response is nil.

Additional Context

This also adds in integration testing for one of the controllers, and the infrastructure to allow this to be done on the other controllers in the future.

@coro coro requested a review from ChunyiLyu April 7, 2021 14:50
Copy link
Contributor

@ChunyiLyu ChunyiLyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome 🚢 Just need to squash before merging.

Copy link
Contributor

@Zerpet Zerpet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hot! 🔥

@coro coro merged commit d8027bb into main Apr 7, 2021
@coro coro deleted the handle-api-errors branch April 7, 2021 16:45
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.

Panic in reconcile loop
3 participants