search

python / the-knights-who-say-ni

CLA enforcement bot for Python organization projects
Apache License 2.0
84 stars 34 forks source link

bpo is returning 200 when there is an error #146

Closed Mariatta closed 6 years ago

Mariatta commented 6 years ago

If we pass in this url to bpo to check for CLA: https://bugs.python.org/user?@template=clacheck&github_names=

The response is <html><head><title>An error has occurred</title></head>\n<body><h1>An error has occurred</h1>\n<p>A problem was encountered processing your request.\nThe tracker maintainers have been notified of the problem.</p>\n</body></html>, with status code 200.

This situation can happen when the PR is made by trusted user (like pyup-io bot). The result is an JSONDecodeError, logged in https://github.com/python/core-workflow/issues/259#issue-330087264

We need to rethink the logic here to handle such situation: https://github.com/python/the-knights-who-say-ni/blob/c0507544200e96d2f30d65e2dae59a6fffe05e21/ni/bpo.py#L17-L28

Mariatta commented 6 years ago

The bpo could instead return something other than status code 200 ...

brettcannon commented 6 years ago

Yep, we can either update b.p.o to return a sane return code and/or put a try/except around the json.loads() call and raise an appropriate exception in that instance (we should probably do both to be as defensive as possible).

Mariatta commented 6 years ago

Fixed in https://github.com/python/bugs.python.org/issues/5. bpo now returns {} instead of an error message.