Problem with substituted GithHubConnector

[The-Huginn]

Hi, It's very possible, it's a (github-api)[https://github.com/hub4j/github-api] issue, yet as this extension does this exact (substitution)[https://github.com/quarkiverse/quarkus-github-api/blob/main/runtime/src/main/java/io/quarkiverse/githubapi/runtime/graal/Substitute_DefaultGitHubConnector.java] I wanted to ask here first.
There is the following error observed.

I see somewhat of a similar issue (here)[https://github.com/OpenLiberty/open-liberty/issues/25279], but this one is for POST requests, so I am not sure how applicable it is here.

Please let me know, whether to open an issue in (github-api)[https://github.com/hub4j/github-api] repository.

The complete stacktrace:

Caused by: org.kohsuke.github.HttpException: Server returned HTTP response code: -1, message: 'null' for URL: https://api.github.com/repos/wildfly/wildfly/pulls/13687/files
at org.kohsuke.github.GitHubClient.interpretApiError(GitHubClient.java:628)
at org.kohsuke.github.GitHubClient.sendRequest(GitHubClient.java:449)
at org.kohsuke.github.GitHubPageIterator.fetch(GitHubPageIterator.java:146)
... 21 more
Caused by: java.io.IOException: /10.129.2.26:38158: GOAWAY received
at java.net.http@21.0.2/jdk.internal.net.http.HttpClientImpl.send(HttpClientImpl.java:964)
at java.net.http@21.0.2/jdk.internal.net.http.HttpClientFacade.send(HttpClientFacade.java:133)
at org.kohsuke.github.extras.HttpClientGitHubConnector.send(HttpClientGitHubConnector.java:72)
at org.kohsuke.github.GitHubClient.sendRequest(GitHubClient.java:431)
... 22 more
Caused by: java.io.IOException: /10.129.2.26:38158: GOAWAY received
at java.net.http@21.0.2/jdk.internal.net.http.Http2Connection.handleGoAway(Http2Connection.java:1204)
at java.net.http@21.0.2/jdk.internal.net.http.Http2Connection.handleConnectionFrame(Http2Connection.java:1018)
at java.net.http@21.0.2/jdk.internal.net.http.Http2Connection.processFrame(Http2Connection.java:851)
at java.net.http@21.0.2/jdk.internal.net.http.frame.FramesDecoder.decode(FramesDecoder.java:155)
at java.net.http@21.0.2/jdk.internal.net.http.Http2Connection$FramesController.processReceivedData(Http2Connection.java:307)
at java.net.http@21.0.2/jdk.internal.net.http.Http2Connection.asyncReceive(Http2Connection.java:778)
at java.net.http@21.0.2/jdk.internal.net.http.Http2Connection$Http2TubeSubscriber.processQueue(Http2Connection.java:1594)
at java.net.http@21.0.2/jdk.internal.net.http.common.SequentialScheduler$LockingRestartableTask.run(SequentialScheduler.java:182)
at java.net.http@21.0.2/jdk.internal.net.http.common.SequentialScheduler$CompleteRestartableTask.run(SequentialScheduler.java:149)
at java.net.http@21.0.2/jdk.internal.net.http.common.SequentialScheduler$SchedulableTask.run(SequentialScheduler.java:207)
at java.net.http@21.0.2/jdk.internal.net.http.common.SequentialScheduler.runOrSchedule(SequentialScheduler.java:280)
at java.net.http@21.0.2/jdk.internal.net.http.common.SequentialScheduler.runOrSchedule(SequentialScheduler.java:233)
at java.net.http@21.0.2/jdk.internal.net.http.Http2Connection$Http2TubeSubscriber.runOrSchedule(Http2Connection.java:1612)
at java.net.http@21.0.2/jdk.internal.net.http.Http2Connection$Http2TubeSubscriber.onNext(Http2Connection.java:1638)
at java.net.http@21.0.2/jdk.internal.net.http.Http2Connection$Http2TubeSubscriber.onNext(Http2Connection.java:1572)
at java.net.http@21.0.2/jdk.internal.net.http.common.SSLTube$DelegateWrapper.onNext(SSLTube.java:210)
at java.net.http@21.0.2/jdk.internal.net.http.common.SSLTube$SSLSubscriberWrapper.onNext(SSLTube.java:492)
at java.net.http@21.0.2/jdk.internal.net.http.common.SSLTube$SSLSubscriberWrapper.onNext(SSLTube.java:295)
at java.net.http@21.0.2/jdk.internal.net.http.common.SubscriberWrapper$DownstreamPusher.run1(SubscriberWrapper.java:316)
at java.net.http@21.0.2/jdk.internal.net.http.common.SubscriberWrapper$DownstreamPusher.run(SubscriberWrapper.java:259)
at java.net.http@21.0.2/jdk.internal.net.http.common.SequentialScheduler$LockingRestartableTask.run(SequentialScheduler.java:182)
at java.net.http@21.0.2/jdk.internal.net.http.common.SequentialScheduler$CompleteRestartableTask.run(SequentialScheduler.java:149)
at java.net.http@21.0.2/jdk.internal.net.http.common.SequentialScheduler$SchedulableTask.run(SequentialScheduler.java:207)
at java.net.http@21.0.2/jdk.internal.net.http.common.SequentialScheduler.runOrSchedule(SequentialScheduler.java:280)
at java.net.http@21.0.2/jdk.internal.net.http.common.SequentialScheduler.runOrSchedule(SequentialScheduler.java:233)
at java.net.http@21.0.2/jdk.internal.net.http.common.SubscriberWrapper.outgoing(SubscriberWrapper.java:232)
at java.net.http@21.0.2/jdk.internal.net.http.common.SubscriberWrapper.outgoing(SubscriberWrapper.java:198)
at java.net.http@21.0.2/jdk.internal.net.http.common.SSLFlowDelegate$Reader.processData(SSLFlowDelegate.java:451)
at java.net.http@21.0.2/jdk.internal.net.http.common.SSLFlowDelegate$Reader$ReaderDownstreamPusher.run(SSLFlowDelegate.java:269)
at java.net.http@21.0.2/jdk.internal.net.http.common.SequentialScheduler$LockingRestartableTask.run(SequentialScheduler.java:182)
at java.net.http@21.0.2/jdk.internal.net.http.common.SequentialScheduler$CompleteRestartableTask.run(SequentialScheduler.java:149)
at java.net.http@21.0.2/jdk.internal.net.http.common.SequentialScheduler$SchedulableTask.run(SequentialScheduler.java:207)
at java.base@21.0.2/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
at java.base@21.0.2/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
... 4 more

[gsmet]

From what I can see GOAWAY is supposed to be sent when the remote server (so the GitHub servers in this case) is shutting down. Are you getting that often?

Now, I'm wondering if we should avoid using HTTP 2 for the API requests, I'm not sure it makes a lot of sense to use HTTP 2 for one-shot requests.

[gsmet]

@The-Huginn if it's in the case of Quarkus GitHub App, could you try this PR and see if it improves things for you: quarkiverse/quarkus-github-app#593 ?

If not, you can use the trick I used anyway when creating your GitHub client.

I'm very interested in your feedback as if it improves things for you, we might have to enforce HTTP 1.1 in the upstream GitHub API.

/cc @bitwiseman FYI

[The-Huginn]

Yes, indeed I am using Quarkus GitHub App. I build it locally and try to deploy my app with this version. Thanks for the hint!

[The-Huginn]

Just a quick update before the holidays. We have the new version deployed for a week and so far no problems.

[gsmet]

@bitwiseman WDYT? I can enforce HTTP 1.1 on my side but it's probably something we should do at the GitHub API level, given there's a good chance it might affect other people.

[bitwiseman]

@gsmet
Interesting. The GitHub docs for moved to HTTP2 in their examples a while back. github/docs#6002

I also found GOAWAY frames mentioned on SO in questions like this one: https://stackoverflow.com/questions/67018337/is-there-any-way-to-handle-http-2-goaway-received-ioexception-in-httpclient-java

That said, it does seem to address the issues and if you're seeing them, they are probably occurring elsewhere as well. If you want to add them to the default constructor Along with a clear comment that would be okay.

We might want to add a warning to the constructor that takes an HttpClient parameter, or otherwise detect the GOAWAY frame error in GitHubClient and return a helpful error message.