errors/3 remove catch alls - #95
Merged
Merged
Conversation
JosteinGj
added this pull request to stack #99
September 15, 2026 09:17
olavgg
force-pushed
the
errors/3-remove-catch-alls
branch
from
September 15, 2026 13:26
f3501e2 to
3fe28e7
Compare
JosteinGj
force-pushed
the
errors/3-remove-catch-alls
branch
from
September 16, 2026 07:40
3fe28e7 to
607dbfa
Compare
JosteinGj
force-pushed
the
errors/3-remove-catch-alls
branch
from
September 16, 2026 07:50
607dbfa to
093ea5d
Compare
JosteinGj
marked this pull request as ready for review
September 16, 2026 07:50
olavgg
approved these changes
Sep 16, 2026
JosteinGj
force-pushed
the
errors/3-remove-catch-alls
branch
from
September 16, 2026 08:02
093ea5d to
d3bf2f9
Compare
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
force-pushed
the
errors/3-remove-catch-alls
branch
from
September 16, 2026 08:11
d3bf2f9 to
e7a45e3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
How it was verified
Checklist
git commit -s), per CONTRIBUTING.md./gradlew buildpasses(see AGENTS.md for which of the two documentation sites it belongs in)