Skip to content

Allow UrlConnectionHttpClient to retry stream-open IOExceptions and NPEs - #7347

Merged
davidh44 merged 4 commits into
masterfrom
hdavidh/urlConnection-fix-retries
Sep 5, 2026
Merged

Allow UrlConnectionHttpClient to retry stream-open IOExceptions and NPEs#7347
davidh44 merged 4 commits into
masterfrom
hdavidh/urlConnection-fix-retries

Conversation

@davidh44

@davidh44 davidh44 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Motivation and Context

Follow-up to #6894. Bare NPEs from HttpURLConnection.getOutputStream() and getInputStream() escaped as runtime exceptions. Wrapped NPEs and checked IOExceptions became UncheckedIOException. RetryableStage catches neither form, so the retry policy never evaluated these failures.

Modifications

  • Preserve checked IOExceptions from stream accessors and response-code checks on these paths.
  • Convert bare NPEs and runtime exceptions directly caused by an NPE from stream accessors to checked IOException, retaining the cause and logging at DEBUG.
  • Catch ProtocolException directly for the Expect: 100-continue workaround. The final-response path retains its existing UncheckedIOException because retrying a completed request could duplicate side effects without recovering the unreadable payload.
  • Update javadoc TODO with the known disconnect() race.

Behavior Changes

  • Stream-accessor and response-content-check failures can now be evaluated for retry.
  • NPE conversion also applies to caller-supplied HttpURLConnection implementations created through create(UrlConnectionFactory).

Testing

  • Added unit tests
  • Existing tests pass

Types of Changes

  • Bug fix (no API surface change, with intentional retry behavior changes described above)
  • New feature

@davidh44
davidh44 requested a review from a team as a code owner September 3, 2026 21:06
@davidh44 davidh44 added the no-api-surface-area-change Indicate there is no API surface area change and thus API surface area review is not required label Sep 3, 2026
@davidh44
davidh44 added this pull request to the merge queue Sep 4, 2026
@davidh44
davidh44 removed this pull request from the merge queue due to a manual request Sep 4, 2026
@davidh44
davidh44 added this pull request to the merge queue Sep 4, 2026
Merged via the queue into master with commit 3705017 Sep 5, 2026
21 of 23 checks passed
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

This pull request has been closed and the conversation has been locked. Comments on closed PRs are hard for our team to see. If you need more assistance, please open a new issue that references this one.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Sep 5, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

no-api-surface-area-change Indicate there is no API surface area change and thus API surface area review is not required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants