Skip to content

Conversation

@erkieh
Copy link

@erkieh erkieh commented Nov 30, 2025

I had failures where one of my used libraries threw an exception which was not serializable. This change should make handling of these better.

@chuck-dbos
Copy link
Contributor

What I absolutely do not want is a behavior that is different in recovery vs. the original run... recovery is the absolute worst time to surprise people because tensions are already running high. As presented, this looks like a difference between original run vs recovery.

Java emphasizes catch by type, and method signatures declaring a throws clause with those types. But here:
Original run -> catch clauses correctly catch the exception that is stated to be thrown by the method signature.
Recovery run -> catch clauses do not catch the exception that was originally thrown because a different RuntimeException derivative is substituted.
(This is in contrast to the current situation, where the original run blows up with an error about nonserializability.)

We can throw a clear error indicating that the return value / thrown exception was not serializable in the original run, and throw the same one in recovery ... that would be OK and probably an improvement. (That is, throw o instead of t in the code presented.) This way, the user could get away with it if they don't care what the exception is, and know to change it to something serializable if they encounter it during development / testing.

Either way, I wouldn't accept any PR that does not include testing of the recovery (reexecution) path.

@erkieh
Copy link
Author

erkieh commented Dec 2, 2025

Would you like to me to work on this PR and implement the changes or you guys have some different ideas?
It does not make difference to me. I can't give promises when I would be able to finish it. But I will be happy either way if the original problem gets resolved.

@devhawk
Copy link
Collaborator

devhawk commented Dec 4, 2025

@erkieh can you share the unserializable error you hit?

@erkieh
Copy link
Author

erkieh commented Dec 5, 2025

It was a org.zalando.problem.ThrowableProblem
which had a non-serializable io.micronaut.problem.HttpStatusType

My biggest problem with that was that once such an exception occurred, the result was not recorded in the workflow status table and the getResult method call was left hanging. This can be reproduced if the AsyncWorkflowTest in the PR is run without my proposed fix in JSONUtil.

@chuck-dbos
Copy link
Contributor

Just like with the return value, the thrown exception will have to be serializable, or we simply can't get correct program behavior. But, we will do something to make it easy to find and fix the programming error. Then the programmer will clearly see early in the development process that the exception is not serializable, and be able to change the step code to interpret the error into one that is.

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.

3 participants