From b59ffc68e0de422dc8060832c4573a303eabfe0b Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Tue, 7 May 2019 10:36:53 +0200 Subject: [PATCH] [FIX] runbot_merge: handle the bot user not being able to comment If the author of a PR has blocked the bot user, commenting on the PR will fail. While comment failure is technically handled in the feedback cron, the cron will simply retry commenting on every run filling the log with useless unactionable garbage. Retrying is the right thing to do in the normal case (e.g. changing tags often has transient failures), but if we figure out we're blocked we might as well just log a warning and drop the comment on the floor, it's unlikely the situation will resolve itself. Couldn't test it, because the block API is a developer preview and I just can't get it to work anyway (404 on /user/blocks even providing the suggested media type). And the way the block is inferred is iffy (based on an error message), the error body doesn't seem to provide any clean / clear cut error code: { "message": "Validation Failed", "errors": [ { "resource": "IssueComment", "code": "unprocessable", "field": "data", "message": "User is blocked" } ], "documentation_url": "https://developer.github.com/v3/issues/comments/#create-a-comment" } No useful headers either. Fixes #127 --- runbot_merge/github.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/runbot_merge/github.py b/runbot_merge/github.py index 4012778e..4b17fa9b 100644 --- a/runbot_merge/github.py +++ b/runbot_merge/github.py @@ -8,6 +8,9 @@ import requests from odoo.tools import topological_sort from . import exceptions, utils +def _is_json(r): + return r and r.headers.get('content-type', '').startswith(('application/json', 'application/javascript')) + _logger = logging.getLogger(__name__) class GH(object): def __init__(self, token, repo): @@ -33,7 +36,7 @@ class GH(object): raise exc(r.text) if r.status_code >= 400: headers = '\n'.join('\t%s: %s' % (h, v) for h, v in r.headers.items()) - if r.headers.get('content-type', '').startswith(('application/json', 'application/javascript')): + if _is_json(r): body = r.json() elif r.encoding is not None: body = utils.shorten(r.text, 200) @@ -76,7 +79,18 @@ class GH(object): return c def comment(self, pr, message): - self('POST', 'issues/{}/comments'.format(pr), json={'body': message}) + # if the mergebot user has been blocked by the PR author, this will + # fail, but we don't want the closing of the PR to fail, or for the + # feedback cron to get stuck + try: + self('POST', 'issues/{}/comments'.format(pr), json={'body': message}) + except requests.HTTPError as r: + if _is_json(r.response): + body = r.response.json() + if any(e.message == 'User is blocked' for e in (body.get('errors') or [])): + _logger.warn("comment(%s:%s) failed: user likely blocked", self._repo, pr) + return + raise _logger.debug('comment(%s, %s, %s)', self._repo, pr, shorten(message)) def close(self, pr, message):