Skip to content

Report first Throwable caught in InMemoryExecutionContext.onError #942

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Feb 5, 2025

Conversation

timtebeek
Copy link
Member

@timtebeek timtebeek commented Feb 4, 2025

What's your motivation?

Makes it easier to troubleshoot recipe execution errors that do not result in source related exceptions.

Anything in particular you'd like reviewers to focus on?

Now we log a warning for the first caught throwable, but do not halt execution or report subsequent warnings.

Anyone you would like to review specifically?

@philippe-granet

Have you considered any alternatives or workarounds?

We could choose to log warnings beyond the first, or even halt execution. That's a more drastic change though, and perhaps good to first, for a while, raise the visibility of these caught throwables.

Any additional context

@timtebeek timtebeek added the enhancement New feature or request label Feb 4, 2025
@timtebeek timtebeek self-assigned this Feb 4, 2025
github-actions[bot]

This comment was marked as off-topic.

@timtebeek timtebeek marked this pull request as ready for review February 4, 2025 23:47
github-actions[bot]

This comment was marked as outdated.

timtebeek and others added 2 commits February 5, 2025 00:53
…java

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions[bot]

This comment was marked as outdated.

Copy link
Contributor

@philippe-granet philippe-granet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great for me. Perhaps you can add a message when option exportDatatables is disabled. This will indicate users they can retrieve all errors by this way

@timtebeek timtebeek merged commit f2c4b2c into main Feb 5, 2025
2 checks passed
@timtebeek timtebeek deleted the report-throwables-caught-in-execution-context branch February 5, 2025 09:52
@timtebeek
Copy link
Member Author

Thanks again for the suggestion! Should help uncover such issues going forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Some errors logged only with Maven DEBUG log level
2 participants