-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
feat: uniform rendering of stack trace from failed DB operations #9364
base: 4.6
Are you sure you want to change the base?
Conversation
6f1ca05
to
79a9c97
Compare
The last time I checked, the full stack track was available: #9338 (comment) and it looks the same for all the drivers. Are there any differences (I may have missed something)? Can you give me an example? |
I am assuming they did not see the full stack trace since Before this PR:
Postgres (stringified error exception)
OCI8 (stringified error exception):
|
After this PR:
|
bf0159a
to
196c605
Compare
196c605
to
40dd62a
Compare
The non-determinism of the tests is really frustrating 🤷🏻♂️ |
Yes. the database package is very thick. I'm afraid to fix him. |
@paulbalandan Ok, I get it now. Thanks! |
👋 Hi, @paulbalandan! |
Description
Closes #9338
All DB drivers except SQL Server logs (with 'error' level) a full stack trace of any failed DB executions via the thrown exception. For SQL Server, the log only includes the message produced by
sqlsrv_errors()
. Moreover, there's inconsistency with regard to Postgre and OCI8 drivers. Sincepg_query()
andoci8_execute()
only throws because of our error handler, the stack trace also includes the error handler (the[internal function]
stack).This PR aims to make consistent the log output of these drivers by using the same trace rendering. This is already achieved by the private
Exceptions::renderBacktrace()
method for the thrown exceptions, so this PR extracts it to a new functionrender_backtrace
and uses that to format the trace. Also, trace coming from the error handler are removed.Checklist: