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

updateCheckRun call fails since 1.128 #1157

Closed
hcoles opened this issue May 26, 2021 · 12 comments · Fixed by #1164
Closed

updateCheckRun call fails since 1.128 #1157

hcoles opened this issue May 26, 2021 · 12 comments · Fixed by #1164
Labels

Comments

@hcoles
Copy link

hcoles commented May 26, 2021

As originally raised in #1156, calls to update check runs that worked in 1.122 now fail once the enum mapping issue is worked around.

The following code

   GitHub github = new GitHubBuilder()
       .withOAuthToken("value supplied by github actions")
       .withEndpoint("value supplied by github actions")
       .build();

   GHCheckRun checkRun = repo.createCheckRun("whatever", "value supplied by github actions").withStatus(GHCheckRun.Status.IN_PROGRESS).create();

   repo.updateCheckRun(checkRun.getId())
      .add(output)
      .withConclusion(GHCheckRun.Conclusion.SUCCESS)
      .withStatus(GHCheckRun.Status.COMPLETED)
      .create();

Runs without issue in 1.122 to 1.127, but fails during the updateCheckRun call from 1.128 onwards with

Caused by: org.kohsuke.github.GHFileNotFoundException: https://api.github.com/repos/XXX/XXX/check-runs/2676567691 {"message":"Not Found","documentation_url":"https://docs.github.com/rest"}
    at org.kohsuke.github.GitHubClient.interpretApiError (GitHubClient.java:482)
    at org.kohsuke.github.GitHubClient.sendRequest (GitHubClient.java:409)
    at org.kohsuke.github.GitHubClient.sendRequest (GitHubClient.java:357)
    at org.kohsuke.github.Requester.fetch (Requester.java:76)
    at org.kohsuke.github.GHCheckRunBuilder.create (GHCheckRunBuilder.java:153)

@gsmet has indicated that this represents an authentication failure, however it seems strange that the call to create the checkrun succeeds, while the call to update it fails using the same credentials.

@gsmet
Copy link
Contributor

gsmet commented May 26, 2021

Ah yeah, if the check creation succeeds, then there might be something else going on.

@bitwiseman bitwiseman added the bug label May 27, 2021
@hcoles hcoles changed the title Authentication regression introduced in 1.128 updateCheckRun call fails since 1.128 May 27, 2021
@hcoles
Copy link
Author

hcoles commented May 27, 2021

Just had a little dig through the git history, and the change that seems to break things is this

8e20f4d

change for #754

It seems that github does not support X-HTTP-Method-Override for this call.

@gsmet
Copy link
Contributor

gsmet commented May 27, 2021

Interesting. That explains why I didn't see it in my GitHub App as I'm using the OkHttp connector.

@hcoles
Copy link
Author

hcoles commented May 27, 2021

Yes, looks like I can workaround this with OkHttp.

@bitwiseman
Copy link
Member

@hcoles

It seems that github does not support X-HTTP-Method-Override for this call.

😭🤦

Hm, so if I rerecorded the tests from #754 they would probably now fail. The question is do we revert #754?

@gsmet
Copy link
Contributor

gsmet commented May 27, 2021

I really am baffled that we are still struggling with this in 2021...

You can report this to GitHub but they are relatively slow to fix things in the APIs so I don't think we have much choice unfortunately.

But that means we need to use the new JDK Http Client introduced in Java 11 for JDK 11+ if we don't want to have recent JDK complaining about this reflection. I think it fixes this, right?

So if I had to go with a plan, that would be:

@hcoles
Copy link
Author

hcoles commented May 27, 2021

@gsmet You could perhaps provide both versions as Java11AndBelowGithubClient and Java12PlusGithubClient or something similar, but less clunky, so it becomes the caller's problem to choose?

I will be switching to OkHttp. I'd rather not have the dependency, but I'll need to support all of 8, 11, and 16.

@hcoles
Copy link
Author

hcoles commented May 27, 2021

Not directly relevant to this issue, but previously I tried calls out from Junit using a personal access token. This now fails when calling the checks api with a message saying I must authenticate as a github app.

Can anyone confirm that this is a behaviour change, or is my memory flakey? Also, is there an easy workaround?

@gsmet
Copy link
Contributor

gsmet commented May 27, 2021

I think we need to make the correct client the automatic default. It can be done using a MR jar (but it's some infrastructure).

As for your issue with a PAT, you are providing it directly or using ~/.github? If you provide it to the API, maybe GH has changed something on their side. I must admit that I develop GitHub Apps so I don't really know.

@gsmet
Copy link
Contributor

gsmet commented May 27, 2021

BTW, if you end up having to develop a GitHub App, my pet project might be of interest to you: https://github.com/quarkiverse/quarkus-github-app .

@hcoles
Copy link
Author

hcoles commented May 27, 2021

I provide the token directly.

Thanks for the link, I've been wanting an excuse to play with quarkus.

@bitwiseman
Copy link
Member

@hcoles
Okhttp also support caching which is generally huge improvement for rate limit usage.

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

Successfully merging a pull request may close this issue.

3 participants