Skip to content
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

Removed catch (Throwable) in OracleSqlDialect to improve error handling and logging #430

Closed

Conversation

dstenger
Copy link
Contributor

@dstenger dstenger commented Oct 8, 2014

Caught throwables were removed (and throw of new IllegalArgumentExceptions without message) to improve the exception handling (now, the "original" exception is thrown instead of a new IllegalArgumentException).
In addition, the logging will be more detailed as log messages can now contain the "original" exception instead of the new IllegalArgumentException.

@dstenger dstenger changed the title Removed catch (Throwable) to improve error handling and logging Removed catch (Throwable) in OracleSqlDialect to improve error handling and logging Oct 8, 2014
@tfr42 tfr42 added this to the 3.4 milestone Oct 13, 2014
@tfr42 tfr42 added enhancement enhancement or improvement tests failing unit or Integration tests fail labels Oct 13, 2014
@MrSnyder
Copy link
Contributor

Tests still failing. Restarted.

@tfr42 tfr42 added the needs rebase PR is not up to date and needs rebase label Mar 4, 2015
@stephanr stephanr self-assigned this Apr 24, 2015
@stephanr
Copy link
Member

The TMC discussed that it would be better to keep logic of createing a new IllegalArgumentException instead of letting the orignal Exception pass.
To keep the original goal of this pull request, the original exception could be passed as cause to the IllegalArgumentException so the trace could be logged and/or used without loosing information.

To achive this, i prepared a pull request at #545 which reflects these changes.

@tfr42 tfr42 added invalid invalid and removed enhancement enhancement or improvement in progress needs rebase PR is not up to date and needs rebase tests failing unit or Integration tests fail labels Apr 30, 2015
@tfr42 tfr42 removed this from the 3.4 milestone Apr 30, 2015
@tfr42
Copy link
Member

tfr42 commented Apr 30, 2015

PR #545 is replaces this one.

@tfr42 tfr42 closed this Apr 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid invalid
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants