Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.The attached patch uses _drupal_decode_exception() to print the exception instead of just getMessage().
This is especially helpful for PDOExceptions, when you can see which db_* call actually triggered the exception also the query that triggered it.
This needs to be improved a bit more, since SimpleTest already displays the file and caller information, so we should override it with that instead of displaying that information twice, differently.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | without_patch.png | 39.69 KB | Berdir |
| #5 | with_patch.png | 107.82 KB | Berdir |
| simpletest_display_exception.patch | 714 bytes | Berdir | |
Comments
Comment #1
boombatower CreditAttribution: boombatower commentedSeems like a good idea.
Curious on the line/function/file display...won't the exception be raised in a different file (usually [or at least can]) then the assert or calling bit of code in the test file. Thus the funciton/line/file display by simpletest gives you the calling bit, and the line/file/function displayed by the assertion in the message text give you the bit of code that raised the assertion? If so then I think we should display both and your curren format seems fine.
Comment #2
boombatower CreditAttribution: boombatower commentedSo I'm fine with it assume what I wrote in #1 is true.
Comment #3
moshe weitzman CreditAttribution: moshe weitzman commentedOh, this is very helpful. +1
Comment #4
Dries CreditAttribution: Dries commentedBerdir wrote: This needs to be improved a bit more, since SimpleTest already displays the file and caller information, so we should override it with that instead of displaying that information twice, differently.
Before committing this patch, I'd like to better understand what needs to be improved. In generally, I agree that we should avoid duplicating this information. It is OK to make bigger changes to SimpleTest, if necessary. It is not really subject to the code freeze. Anyway, let's see what else is required here.
Comment #5
BerdirI don't think that there are bigger chances necessary here.
What I was trying to say is that simpletest displays the location where an exception was thrown. With my patch, that information is also displayed as part of the actual message. See attached screenshot.
The difference is however, that _drupal_decode_exception() contains special handling for PDO-Exception which displays the location where the query was actually executed and also displays the full query. This is really helpful, because some PDO-Exceptions are a) quite cryptic and b) you have no clue which query actually failed.
Now, imho, the only question is if we want to..
a) Keep it as it with the patch, see screenshot
b) Display the location only once.
I like a) more, because in some cases, you want to know where exactly the exception was thrown, for example, when there are bugs in DBTNG itself.
Comment #6
moshe weitzman CreditAttribution: moshe weitzman commentedNice screenies. I think it is pretty clear that 'with_patch' is superior.
On a related note, see #711108: Richer exception reporting for watchdog(). Perhaps berdir can lend a hand there. That deals with PDO exceptions outside of simpletest.
Comment #7
Dries CreditAttribution: Dries commentedClearly better! Committed to CVS HEAD.