Skip to content

errors/3 remove catch alls - #95

Merged
JosteinGj merged 1 commit into
mainfrom
errors/3-remove-catch-alls
Sep 16, 2026
Merged

JosteinGj merged 1 commit into
mainfrom
errors/3-remove-catch-alls

Conversation

@JosteinGj

Copy link
Copy Markdown
Contributor

What this changes

How it was verified

Checklist

  • Pull requests this one depends on are named above and merged first, or there are none
  • Commits are signed off (git commit -s), per CONTRIBUTING.md
  • ./gradlew build passes
  • Behaviour visible outside this repo is reflected in the documentation, or does not need to be
    (see AGENTS.md for which of the two documentation sites it belongs in)

@JosteinGj
JosteinGj added this pull request to stack #99 September 15, 2026 09:17
@olavgg
olavgg force-pushed the errors/3-remove-catch-alls branch from f3501e2 to 3fe28e7 Compare September 15, 2026 13:26
@JosteinGj
JosteinGj force-pushed the errors/3-remove-catch-alls branch from 3fe28e7 to 607dbfa Compare September 16, 2026 07:40
@JosteinGj
JosteinGj force-pushed the errors/3-remove-catch-alls branch from 607dbfa to 093ea5d Compare September 16, 2026 07:50
@JosteinGj
JosteinGj marked this pull request as ready for review September 16, 2026 07:50
@JosteinGj
JosteinGj requested a review from olavgg as a code owner September 16, 2026 07:50
Base automatically changed from errors/2-advices to main September 16, 2026 08:02
@JosteinGj
JosteinGj force-pushed the errors/3-remove-catch-alls branch from 093ea5d to d3bf2f9 Compare September 16, 2026 08:02
BREAKING: a failure that was reported as 500 with a bare string now arrives as
the problem its type describes — 400, 403, 404, 409 or 503 — in RFC 9457 shape.

Removing the 66 redundant catches alone would have made things worse. Each sat
above a catch (PulsarClientException | RuntimeException) answering 500, and
ConstraintViolationException, BadRequestException and DuplicateDataException all
extend RuntimeException: delete their handlers and they land in the net below
and become 500s instead of reaching the new advices. The catch-alls had to go
first.

They were already being fought. Thirty-nine of them across twelve controllers,
and fifty-two explicit "throw e;" rethrows existing only to escape them — two
carrying comments saying exactly that, one about NamingPolicyViolationException
being flattened and one about a limit refusal becoming a 500. Removing the nets
makes the rethrows unnecessary, and the specific catches with them: 56
try-statements rewritten, and every "Internal programming error." gone.

Pulsar publish failures were caught by those nets. They are checked, so the
handlers declare them, and MessagingUnavailableExceptionHandler answers 503
rather than 500 — the request was well formed and retrying is right.

NodeFamilyParityTest F9 is inverted rather than deleted. It required a local
catch for DuplicateDataException because without one the catch-all produced a
bare 500; it now requires the absence of one, since a local catch re-implements
the advice and is free to drift from it. F9b pins the advice it now relies on.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: jgjesdal <jostein@intellistream.ai>
@JosteinGj
JosteinGj force-pushed the errors/3-remove-catch-alls branch from d3bf2f9 to e7a45e3 Compare September 16, 2026 08:11
@JosteinGj
JosteinGj merged commit cc6e34a into main Sep 16, 2026
8 checks passed
@JosteinGj
JosteinGj deleted the errors/3-remove-catch-alls branch September 16, 2026 08: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.

2 participants