Skip to content

fix: close non-success responses in error handler - #879

Open
sylvesterkaczmarek wants to merge 2 commits into
openai:mainfrom
sylvesterkaczmarek:fix/close-error-responses
Open

fix: close non-success responses in error handler#879
sylvesterkaczmarek wants to merge 2 commits into
openai:mainfrom
sylvesterkaczmarek:fix/close-error-responses

Conversation

@sylvesterkaczmarek

Copy link
Copy Markdown

Summary

Make the shared HTTP error handler close every non-2xx HttpResponse before propagating a typed service exception or an error-body parsing failure.

Fixes #745.

Problem

Generated service methods generally call the shared errorHandler.handle(response) before entering their later response.use { ... } parsing block.

For 2xx responses, that is correct: the handler returns the response and the service owns its eventual close.

For non-2xx responses, however, errorHandler currently reads the status/headers/body and throws a typed exception without closing the response. Because the exception is thrown before the service reaches its own use block, the non-success response can remain open.

The same leak can occur if the supplied error-body handler itself throws while parsing the response.

Fix

Keep successful response ownership unchanged, but put the complete non-success path inside response.use { ... }:

  1. return 2xx responses directly and leave them open for the caller;
  2. for every other status, enter response.use;
  3. capture headers and parse the error body;
  4. construct and throw the same status-specific exception as before.

use closes the response whether typed exception construction succeeds or error-body parsing throws first.

The status-to-exception mapping is unchanged:

  • 400 -> BadRequestException
  • 401 -> UnauthorizedException
  • 403 -> PermissionDeniedException
  • 404 -> NotFoundException
  • 422 -> UnprocessableEntityException
  • 429 -> RateLimitException
  • 5xx -> InternalServerException
  • all other non-2xx statuses -> UnexpectedStatusCodeException

Regression coverage

Added focused tests that verify:

  • representative status codes from every error branch close the response;
  • a failure thrown by the error-body handler still closes the response and propagates the original failure;
  • a 200 response is returned unchanged and remains open for caller-owned parsing.

Validation

  • branch is based directly on current upstream main at 6a46d024ed67e2be889a4c729837a664d5e7902c;
  • branch is 0 commits behind upstream;
  • changes are limited to the shared error handler and focused ownership regressions;
  • successful-response parsing and exception type selection are unchanged.

Full repository validation is left to GitHub Actions because this environment does not have a complete local checkout/toolchain for the repository.

Risk

Low. The ownership change applies only to responses that are already being rejected. Successful responses remain open exactly as before, and callers still receive the same typed exceptions for each status class.

@sylvesterkaczmarek
sylvesterkaczmarek requested a review from a team as a code owner August 18, 2026 13:08
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.

Non-2xx responses are not closed when error handling throws

1 participant