Skip to content

Conversation

vitarb
Copy link
Contributor

@vitarb vitarb commented Nov 13, 2020

This change removes illegal reflective access to the cause field in the CheckedExceptionWrapper replacing it with shallow unwrapping of the exception plus full unwrapping during serialization.

The reason we need to keep shallow unwrapping instead of getting rid of it entirely is because there are a few places in the pre-serialization code that either log or conduct actions based on the client exception. Look at POJOWorkflowImplementationFactory and POJOActivityTaskHandler as examples.

One tradeoff that we have to make here is whether or not we want to eliminate a possibility of client seeing CheckedExceptionWrapper as current approach doesn't guarantee that and if there are multiple layers of wrappers inner layers might be visible in the log, logged by POJOActivityTaskHandler. Preferred way to solve it could be by transforming that exception to Failure before logging, alternatively we could reimplement unwrap by serializing to failure and back, which would eliminate all exception wrappers in the process.

@vitarb vitarb requested a review from mfateev November 13, 2020 19:52
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