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] Add missing re-raise on ClientResponseError #1806

Merged
merged 2 commits into from
Oct 18, 2023

Conversation

timgrein
Copy link
Contributor

@timgrein timgrein commented Oct 18, 2023

Closes https://github.com/elastic/enterprise-search-team/issues/6067

This fixes an issue, where a non-401 error leads to an implicit return None leading to this error: TypeError: cannot unpack non-iterable NoneType object.

The code inside post returns an implicit None, if a ClientResponseError is raised with a status != 401:

except ClientResponseError as exception:
            if exception.status == 401:
                raise UnauthorizedException(
                    "Your Github token is either expired or revoked. Please check again."
                ) from exception
                # implicit return None
        except Exception:
            raise

This leads to the error described above, when trying to unpack it in _remote_validation:

_, headers = await self.github_client.post(  # pyright: ignore
            query_data=query_data, need_headers=True
        )

Fixed by re-raising the original exception, which contains useful information.

Checklists

Pre-Review Checklist

  • this PR has a meaningful title
  • this PR links to all relevant github issues that it fixes or partially addresses
  • if there is no GH issue, please create it. Each PR should have a link to an issue
  • this PR has a thorough description
  • Covered the changes with automated tests
  • Tested the changes locally
  • Added a label for each target release version (example: v7.13.2, v7.14.0, v8.0.0)

Release Note

Fixes an issue where the Github Connector might receive 4xx errors during the sync, and fail without a helpful error message or log.

@timgrein timgrein merged commit b556be5 into main Oct 18, 2023
@timgrein timgrein deleted the tim/github-connector-missing-re-raise branch October 18, 2023 09:33
@github-actions
Copy link

💔 Failed to create backport PR(s)

The backport operation could not be completed due to the following error:
There are no branches to backport to. Aborting.

The backport PRs will be merged automatically after passing CI.

To backport manually run:
backport --pr 1806 --autoMerge --autoMergeMethod squash

@seanstory
Copy link
Member

💚 All backports created successfully

Status Branch Result
8.11

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

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

Successfully merging this pull request may close these issues.

3 participants