allthingssecurity commented on PR #29525:
URL: https://github.com/apache/camel/pull/29525#issuecomment-6767359567
Thanks orpiske. I tried the approach we agreed on, and went with the second
option instead. Here is why.
Failing the exchange with a `VirtualMachineError` without the error handler
works with the flags Camel already has (failure handled, not handled by the
error handler, route stop). But two parts did not hold:
- Throwing the `Error` on to the calling thread after that brings back the
hang this PR fixes. The reactive executor's worker catches only `Exception`, so
the continuation the callback queued is abandoned: the exchange stays inflight
and the context stop waits its full timeout. With a dead letter channel or
`onException`, `RedeliveryErrorHandler`'s redelivery task catches the `Error`
after the callback and only logs a WARN.
- The marks are lost where split or multicast copy the exchange back to the
parent, so a `doCatch` around a split would still catch it.
So a6650ba6c575 leaves `VirtualMachineError` exactly as before and limits
the PR to the other `Error`s:
| Thrown by a plain `Processor` | What happens |
|---|---|
| An `Error` that is not a `VirtualMachineError` (`AssertionError`,
`LinkageError`, `ExceptionInInitializerError`, a custom `Error`) | The exchange
fails with it, and the error handler handles it as any other failure
(redelivery, `onException`, `doCatch`, dead letter channel) |
| A `VirtualMachineError` (`OutOfMemoryError`, `StackOverflowError`,
`InternalError`) | Not changed: the adapters do not catch it, and it is thrown
on after the callback. Camel's error handling does not get it |
- The three adapters now have `catch (VirtualMachineError e) { throw e; }`
ahead of the `catch (Throwable e)`.
- The 4.24 upgrade guide entry and a new section on the error handler page
have this table. They also say what an application with strict reliability
needs can do: `-XX:+ExitOnOutOfMemoryError` or `-XX:+CrashOnOutOfMemoryError`
for an `OutOfMemoryError` the JVM raises.
- Claus's two notes are in: `sendBody`/`requestBody` throw unless the
failure was handled, and `request`/`asyncSend` are listed next to `send`.
- Tests: `ProcessorThrowsErrorTest` now also checks that an
`OutOfMemoryError` or a `StackOverflowError` from a processor, a bean that is a
`Processor`, and a producer that is not an `AsyncProcessor` is not given to
`onException`, `doCatch`, redelivery or the dead letter channel, as on main.
12/12, 10 runs in one JVM 120/120. The camel-core processor, bean, sanitizer,
`*Error*` and `*Throwable*` tests: 4299 run, 0 failures.
If you would like Camel to handle a `VirtualMachineError` at the engine
level (the reactive executor and the error handlers, so the exchange fails and
the `Error` still reaches the thread), I can open a separate JIRA for that.
_Claude Code on behalf of allthingssecurity_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
