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 PR deserialization bug #860

Merged
merged 2 commits into from Jan 29, 2020
Merged

Fix PR deserialization bug #860

merged 2 commits into from Jan 29, 2020

Conversation

@noizwaves
Copy link
Contributor

noizwaves commented Jan 29, 2020

We noticed an issue where certain GitHub webhook requests to Bors returned 500 status codes and returned Internal Server Error:

Screen Shot 2020-01-28 at 5 04 14 PM

We've identified the root cause for the errors was in the JSON parsers for the BorsNG.GitHub.Pr and how it handles a "sometimes present" field named mergeable. It turns out the mergeable field is only present on GitHub v3 REST API Pull Request responses and some webhook events (only the PullRequestEvent / pull_request event type). The existing logic required the field be present ALWAYS.

The fix

When the field is missing from the json argument, we give it a nil value. We also added a simple unit test for the BorsNG.GitHub.Pr module.

Misc

We think this PR also fixes #855

@notriddle

This comment has been minimized.

Copy link
Member

notriddle commented Jan 29, 2020

bors r+

bors bot added a commit that referenced this pull request Jan 29, 2020
Merge #860
860: Fix PR deserialization bug r=notriddle a=noizwaves

We noticed an issue where certain GitHub webhook requests to Bors returned `500` status codes and returned `Internal Server Error`:

![Screen Shot 2020-01-28 at 5 04 14 PM](https://user-images.githubusercontent.com/1007983/73316509-50b1af80-41f0-11ea-8aef-c569f6218712.png)

We've identified the root cause for the errors was in the JSON parsers for the `BorsNG.GitHub.Pr` and how it handles a "sometimes present" field named `mergeable`. It turns out the `mergeable` field is only present on GitHub v3 REST API Pull Request responses and *some* webhook events (only the `PullRequestEvent` / `pull_request` event type). The existing logic required the field be present ALWAYS.

## The fix
When the field is missing from the json argument, we give it a `nil` value. We also added a simple unit test for the `BorsNG.GitHub.Pr` module.

## Misc
We think this PR also fixes #855

Co-authored-by: Adam Neumann <adam.neumann@gusto.com>
@bors

This comment has been minimized.

Copy link
Contributor

bors bot commented Jan 29, 2020

Build succeeded

@bors bors bot merged commit fc89acf into bors-ng:master Jan 29, 2020
3 checks passed
3 checks passed
Community-TC (pull_request) TaskGroup: success
Details
Travis CI - Pull Request Build Passed
Details
bors Build succeeded
Details
notriddle added a commit to bors-ng/bors-ng.github.io that referenced this pull request Feb 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.