Skip to content

GitHub plugin 1.0.2: fix the connection leak and the swallowed status - #3

Merged
Klaas-Ritense merged 2 commits into
mainfrom
release/1.0.2
Sep 16, 2026
Merged

Klaas-Ritense merged 2 commits into
mainfrom
release/1.0.2

Conversation

@Klaas-Ritense

Copy link
Copy Markdown
Member

Two faults found by driving a real process against a real repository, both of which present as something other than what they are.

The plugin stopped reaching GitHub after five calls

RestClient.exchange was passed close = false, which makes closing the response the caller's job — and nothing was doing it, so every call leaked the connection it borrowed. Apache's pool lends five per route and Valtimo waits five seconds for one, so the sixth call and everything after it died with ConnectionRequestTimeoutException: Timeout deadline: 5000 MILLISECONDS until the application was restarted.

A process running one or two actions never noticed. A process that walks a queue — the pull requests, then per pull request its reviews, its checks and its runs — hit it every time, somewhere in the middle, with an error that named a timeout and said nothing about GitHub.

RestClient closes in a finally, so this covers the throwing path too. That is the one that matters: a repository that refuses a write refuses it every time, and each attempt was leaking.

Create label works on the second run

Create label treats "this label already exists" as success by looking the existing label up. That only works if it can see GitHub's 422, and it could not: Valtimo's LoggingRestClientCustomizer reads the head of an error response and raises HttpClientErrorException before the exchange function runs, so GitHubException — and its status — was never built. A process whose first step creates its working label ran exactly once and failed on every run after that.

The status now survives whichever half of the stack noticed the refusal. The reason does not: that customizer builds its exception from the status line alone and discards the body it has just read, so a rejected write still reports GitHub responded 422 without naming the field. Recovering that needs a change in Valtimo — it already holds responseBodyHead and could pass it to the exception.

Also

  • The published jar carried Spring Boot's plain classifier while the POM points at the jar without one. github-plugin-1.0.1.jar genuinely does not exist on Maven Central — only -plain.jar — so no Maven consumer could resolve it. Gradle consumers were saved by the module metadata.
  • .gitignore had /deployment/**, which never matches frontend/deployment/, so the frontend build output was not ignored.

Verification

Both fixes were found and confirmed against a live repository, not by inspection. The leak surfaced mid-sweep while walking a queue of pull requests; the 422 surfaced on the second run of a process that creates its own label. :backend:plugin:test, ktlintCheck and npm run lint pass.

Two faults found by driving a real process against a real repository, both of
which look like something other than what they are.

The client borrowed a pooled connection per call and never gave it back:
`RestClient.exchange` was passed `close = false`, which makes closing the
caller's job, and nothing was doing it. Apache lends five per route and Valtimo
waits five seconds for one, so the sixth call and everything after it died with
`ConnectionRequestTimeoutException` until the application was restarted. One or
two actions never noticed; a process that walks a queue hit it every time.

A refusal arrived as Spring's exception rather than the plugin's, because
Valtimo's logging interceptor reads the head of an error response and raises
before the exchange function can. `GitHubException.status` was therefore never
set, and `createLabel`, which treats 422 as "the label is already there", could
not see the 422 — so a process whose first step creates its working label ran
exactly once and failed on every run after that. The status now survives; the
body does not, and cannot, because that interceptor drops it.

Also: the published jar carried Spring Boot's `plain` classifier while the POM
pointed at the jar without one, so `github-plugin-1.0.1.jar` does not exist on
Maven Central and no Maven consumer could resolve it. And `/deployment/**` never
matched `frontend/deployment/`, so the frontend build output was not ignored.
CodeQL's autobuild runs `./gradlew testClasses` under its tracer, and the Kotlin
daemon inherited `org.gradle.jvmargs`' 1 GB ceiling. That is enough untraced —
the test job compiles the same code in under three minutes — but the traced build
spent 23 minutes and then died with `OOMErrorException: Not enough memory to run
compilation`, which is the one thing standing between this branch and a green
scan.
@Klaas-Ritense
Klaas-Ritense merged commit 8f65611 into main Sep 16, 2026
6 checks passed
@Klaas-Ritense
Klaas-Ritense deleted the release/1.0.2 branch September 16, 2026 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant