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

GitHub is sending us PR synchronize event for a PR that does not exist. #109

Open
Mariatta opened this issue Jun 11, 2018 · 4 comments
Open

Comments

@Mariatta
Copy link
Member

Same issue as described in python/the-knights-who-say-ni#148.
Bedevere is attempting to apply a label to such PR, causing gidgethub.BadRequest: Not Found error.

bedevere/bedevere/bpo.py

Lines 35 to 42 in 7cd16ef

@router.register("pull_request", action="opened")
@router.register("pull_request", action="synchronize")
async def set_status(event, gh, *args, **kwargs):
"""Set the issue number status on the pull request."""
issue_number_found = ISSUE_RE.search(event.data["pull_request"]["title"])
if not issue_number_found:
issue = await util.issue_for_PR(gh, event.data["pull_request"])
status = SKIP_ISSUE_STATUS if util.skip("issue", issue) else FAILURE_STATUS

Perhaps we can catch the error here, so I won't keep getting error alerts.
Thanks.

@brettcannon
Copy link
Member

Yep, if GitHub gives us bad data then we should log that fact and then move on.

@Mariatta
Copy link
Member Author

🍝 (copy pasta 😄)

Update from GitHub:

PR 5581 exists, but it just has been marked as "suspicious/spammy", so we and the bots can't access it. The PR was still open and we were receiving the "synchronize" events, breaking the bots.

I have asked GitHub to close the PR, and it has been closed now.

We can still improve the code though if this ever happens again :)

@lysnikolaou
Copy link
Member

lysnikolaou commented Oct 12, 2019

@brettcannon @Mariatta How do you think the right approach for this would be? Should we wrap every call to post with a try/except, where we log the event (which would be a bigger change) or maybe just do that for the code in set_status?

@brettcannon
Copy link
Member

@lysnikolaou this definitely isn't a priority to fix, but yes, a well-documents try/except with a tight except clause would be the way to handle it.

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

No branches or pull requests

3 participants