GithubIntegration.get_access_token broken?

[boegel]

I'm hitting problems with a GitHub App that is using GithubIntegration.get_access_token.
It was working fine before (and running for 2 months), but a restart of the app after some code changes now leads to trouble.

When using PyGithub 1.57 with Python 3.9 (or earlier, I was still running PyGithub 1.55 with Python 3.6 when first hitting this), I get:

Traceback (most recent call last):
  File "/usr/lib64/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/lib64/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/mnt/shared/home/boegel/eessi-bot-software-layer/eessi_bot_event_handler.py", line 412, in <module>
    main()
  File "/mnt/shared/home/boegel/eessi-bot-software-layer/eessi_bot_event_handler.py", line 396, in main
    github.connect()
  File "/mnt/shared/home/boegel/eessi-bot-software-layer/connections/github.py", line 59, in connect
    return Github(get_token().token)
  File "/mnt/shared/home/boegel/eessi-bot-software-layer/connections/github.py", line 43, in get_token
    _token = github_integration.get_access_token(installation_id)
  File "/mnt/shared/home/boegel/.local/lib/python3.9/site-packages/github/MainClass.py", line 855, in get_access_token
    raise GithubException.UnknownObjectException(
TypeError: __init__() missing 1 required positional argument: 'headers'

After updating to the latest PyGithub 1.58.2 with Python 3.9, I get:

Traceback (most recent call last):
  File "/usr/lib64/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/lib64/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/mnt/shared/home/boegel/eessi-bot-software-layer/eessi_bot_event_handler.py", line 412, in <module>
    main()
  File "/mnt/shared/home/boegel/eessi-bot-software-layer/eessi_bot_event_handler.py", line 396, in main
    github.connect()
  File "/mnt/shared/home/boegel/eessi-bot-software-layer/connections/github.py", line 59, in connect
    return Github(get_token().token)
  File "/mnt/shared/home/boegel/eessi-bot-software-layer/connections/github.py", line 43, in get_token
    _token = github_integration.get_access_token(installation_id)
  File "/mnt/shared/home/boegel/.local/lib/python3.9/site-packages/github/GithubIntegration.py", line 135, in get_access_token
    headers, response = self.__requester.requestJsonAndCheck(
  File "/mnt/shared/home/boegel/.local/lib/python3.9/site-packages/github/Requester.py", line 398, in requestJsonAndCheck
    return self.__check(
  File "/mnt/shared/home/boegel/.local/lib/python3.9/site-packages/github/Requester.py", line 423, in __check
    raise self.__createException(status, responseHeaders, output)
github.GithubException.UnknownObjectException: 404 {"message": "Not Found", "documentation_url": "https://docs.github.com/rest/reference/apps#create-an-installation-access-token-for-an-app"}

Is GithubIntegration.get_access_token known to be broken currently, or am I overlooking something?

I have already replaced the webhook secret, but that doesn't make a difference.

I looked into #2431, but that seems like a different problem.

[EnricoMi]

If after your code changes, your code does not work with the pygithub version that worked for 2 months, than your code changes might have caused it. How do you create github_integration? What value does installation_id have?

[boegel]

@EnricoMi Thanks for the reply, and the initial hint.

I was indeed not using the correct installation_id, which explains the 404 I was getting.
To be frank, the error message could be a bit more clear, shouldn't PyGithub be able to report that the installation_id is incorrect?

After making sure I was using the correct integration_id (to create the GithubIntegration instance), and the correct installation_id when calling get_access_token, I was running into another problem:

github.GithubException.GithubException: 401 {"message": "A JSON web token could not be decoded", "documentation_url": "https://docs.github.com/rest"}

This made me scratch my head a bit again, but I then realized that I wasn't using the correct private key when creating the GithubIntegration instance.

Once that was fixed, it's working as expected again (with PyGithub 1.58.2 and Python 3.9).

Thanks a lot for giving me a push in the right direction!

I think this shows that error reporting could be a bit more informative, would you agree?

You basically need to make sure that the integration_id, private_key, and installation_id are in sync. While that makes perfect sense of course, it isn't easy to deduce that from the error messages I was presented.

Do you think there's room for improvement there?
I'm happy to try and take a stab at making a pull request myself, if only to help my future self which is no doubt to hit problems like this again in the not-so-distant future...

[EnricoMi]

Method GithubIntegration.get_access_token should check that integration_id is an int. This type of check is pretty standard throughout the code base, but is missing here.

assert isinstance(installation_id, (int, str)), installation_id

What other checks would you suggest?

[boegel]

integration_id was an int in my case, but the value was wrong (I was switching between different GitHub Apps & installations to test my code with).

I'm not sure if it's possible, or how easy it would be, but PyGithub could check:

• When the GithubIntegration instance is created, PyGithub should do a sanity check on both the integration_id (does it exist?) and the private key (does it match with the specified integration_id), and produce a meaningful error if there's a mismatch.
• If the installation_id exists, and produce a meaningful error message when it doesn't ("GitHub App installation with ID 12345 does not exist");

Again, I'm not sure how easy/hard this is, but more meaningful error messages would definitely have helped me to understand where I was wrong.

[EnricoMi]

The code base in this project tries to minimize Github API calls. All that methods can check are the provided arguments. Checking if objects exist (like installation id) should be redundant, because a 404 error is raised as soon as that invalid id is used. So UnknownObjectException should be good enough.

Your first error (TypeError: __init__() missing 1 required positional argument: 'headers') is a known bug and should be fixed: #2079 #2324