From ca2742a12c5dbbfdd531c5eaf843e4bcd101e343 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Tue, 27 Jul 2021 15:49:49 +0200 Subject: [PATCH] [IMP] forwardport: error handling when creating PR Don't try to parse the response as JSON in the error case(s): if the errors are bad enough github can return complete non-parseable garbage. Only access the "text" property (response body, decoded, but unparsed) in error cases, only parse in the success case. Also avoid reusing variables for completely different values, even if they're of the same type, especially if they can overlap. fixes #470 --- forwardport/models/project.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/forwardport/models/project.py b/forwardport/models/project.py index b360dcc1..7b767d20 100644 --- a/forwardport/models/project.py +++ b/forwardport/models/project.py @@ -671,23 +671,20 @@ class PullRequests(models.Model): del pr_data['draft'] r = gh.post(url, json=pr_data) - results = r.json() - if not (200 <= r.status_code < 300): + if not r.ok: _logger.warning("Failed to create forward-port PR for %s, deleting branches", pr.display_name) # delete all the branches this should automatically close the # PRs if we've created any. Using the API here is probably # simpler than going through the working copies for repo in self.mapped('repository'): - r = gh.delete('https://api.github.com/repos/{}/git/refs/heads/{}'.format(repo.fp_remote_target, new_branch)) - if r.ok: + d = gh.delete('https://api.github.com/repos/{}/git/refs/heads/{}'.format(repo.fp_remote_target, new_branch)) + if d.ok: _logger.info("Deleting %s:%s=success", repo.fp_remote_target, new_branch) else: - _logger.warning("Deleting %s:%s=%s", repo.fp_remote_target, new_branch, r.json()) - raise RuntimeError("Forwardport failure: %s (%s)" % (pr.display_name, results)) + _logger.warning("Deleting %s:%s=%s", repo.fp_remote_target, new_branch, d.text) + raise RuntimeError("Forwardport failure: %s (%s)" % (pr.display_name, r.text)) - r = results - - new_pr = self._from_gh(r) + new_pr = self._from_gh(r.json()) _logger.info("Created forward-port PR %s", new_pr) new_batch |= new_pr