_drupal_exception_handler() attempts to log and render uncaught exceptions.
If another exception is thrown during this process, then the following error is printed:
Fatal error: Exception thrown without a stack frame in Unknown on line 0
This doesn't provide much information about what happened.
The attached patch outputs details of both the original exception and the additional exception, without risking another exception being thrown.
This is analogous to how Apache handles errors that are generated when generating an error page.
Comment | File | Size | Author |
---|---|---|---|
#51 | drupal.handle_uncaught_exceptions_742246_51.patch | 7.38 KB | rfay |
#48 | 742246-48-drupal-catch_unhandled_exceptions-D7.patch | 7.67 KB | alexanderpas |
#43 | catch-session-exceptions.patch | 7.55 KB | jbrown |
#41 | catch-session-exceptions_42.patch | 7.55 KB | aspilicious |
#39 | catch-session-exceptions.patch | 7.55 KB | jbrown |
Comments
Comment #1
catchSubscribing. This makes MySQL fatal errors impossible to debug, so bumping to critical.
Comment #2
BerdirSee also #710142: Handle exceptions in shutdown functions, maybe we should report these exceptions in a similiar way?
That could mean:
- Use _drupal_decode_exception as in the other issue to display the exception instead of simply printing them
- Check the error_level in _drupal_shutdown_function() as you did here
Comment #3
jbrown CreditAttribution: jbrown commentedOkay - here's a patch, but I have a better idea.
Comment #4
jbrown CreditAttribution: jbrown commentedActually, my other idea was stupid. Please review the patch.
Comment #5
Anonymous (not verified) CreditAttribution: Anonymous commentedjust applied this patch, and it printed the following, which is an improvement, but seems there is an underlying issue in the db layer:
Comment #6
Anonymous (not verified) CreditAttribution: Anonymous commentedactually, there isn't, it was an error thrown in theme('maintenance_page', ...) because of some of my yet-to-be-ported-to-D7 code.
Comment #7
catchFixes the issue, the way it does it is pretty self-documenting. RTBC. Would be nice not to see any more "Exception thrown without a stack frame in line 0".
Comment #8
Anonymous (not verified) CreditAttribution: Anonymous commentedjust a note to say this is making it harder to fix update bugs: #757214: D6->D7 update.php failure: Fatal error: Class 'MergeQuery_mysql'.
Comment #9
Dries CreditAttribution: Dries commentedCould we get a quick reroll minus the whitespace?
Comment #10
jbrown CreditAttribution: jbrown commentedComment #11
jbrown CreditAttribution: jbrown commentedsorry - let me try that again
Comment #12
jbrown CreditAttribution: jbrown commentedComment #13
jbrown CreditAttribution: jbrown commentedComment #14
rfayComment #15
bcn CreditAttribution: bcn commentedOne last bit of whitespace removed, otherwise it's the same patch.
Comment #16
Anonymous (not verified) CreditAttribution: Anonymous commentedlooks good to me.
Comment #17
webchickSince Dries just asked for a no-whitespace re-roll, and this fixes the most flamingly critical bug I can think of, committed to HEAD. Great sleuthing work, jbrown!
Comment #18
webchickOh, though one question. Is it possible to write tests for this at all so we don't EVER have this problem again?
Comment #19
jbrown CreditAttribution: jbrown commentedGreat!
I'll look into testing this.
Comment #20
tobiasbI get this message in all tests now, when the default language isn't english. (here german)
Ein AJAX-HTTP-Fehler ist aufgetreten HTTP-Rückgabe-Code: 500 Im Folgenden finden Sie Debugging-Informationen. Pfad: /drupal7/batch?render=overlay&id=23&op=do StatusText: Service unavailable (with message) ResponseText: Additional uncaught exception thrown while handling exception.OriginalPDOException: SQLSTATE[42S02]: Base table or view not found: 1146 Table 'drupal7.simpletest253844locales_source' doesn't exist: SELECT s.source, s.context, t.translation, t.language FROM {locales_source} s LEFT JOIN {locales_target} t ON s.lid = t.lid AND t.language = :language WHERE s.textgroup = 'default' AND s.version = :version AND LENGTH(s.source) de [:version] => 7.0-dev ) in locale() (line 656 of F:\xampplite\htdocs\drupal7\modules\locale\locale.module).AdditionalPDOException: SQLSTATE[42S02]: Base table or view not found: 1146 Table 'drupal7.simpletest253844locales_source' doesn't exist: SELECT s.lid, t.translation, s.version FROM {locales_source} s LEFT JOIN {locales_target} t ON s.lid = t.lid AND t.language = :language WHERE s.source = :source AND s.context = :context AND s.textgroup = 'default'; Array ( [:language] => de [:source] => %type: %message in %function (line %line of %file). [:context] => ) in locale() (line 674 of F:\xampplite\htdocs\drupal7\modules\locale\locale.module). Fatal error: Exception thrown without a stack frame in Unknown on line 0
Comment #21
jbrown CreditAttribution: jbrown commentedYour checkout of locale.inc is not HEAD as the line numbers are wrong - I presume you are doing development in locale.inc .
The double exception reporting is working - its reporting an error. You seem to have some other bug.
I don't know where the final 'Fatal error: Exception thrown without a stack frame in Unknown on line 0' is coming from though. Its probably something to do with it occurring in simpletest.
Comment #22
tobiasbSame with a clean HEAD and only enabled german as default language.
An AJAX HTTP error occurred. HTTP Result Code: 500 Debugging information follows. Path: /drupal7/batch?render=overlay&id=2&op=do StatusText: Service unavailable (with message) ResponseText: Additional uncaught exception thrown while handling exception.OriginalPDOException: SQLSTATE[42S02]: Base table or view not found: 1146 Table 'drupal7.simpletest748206locales_source' doesn't exist: SELECT s.source, s.context, t.translation, t.language FROM {locales_source} s LEFT JOIN {locales_target} t ON s.lid = t.lid AND t.language = :language WHERE s.textgroup = 'default' AND s.version = :version AND LENGTH(s.source) de [:version] => 7.0-dev ) in locale() (line 656 of F:\xampplite\htdocs\drupal7\modules\locale\locale.module).AdditionalPDOException: SQLSTATE[42S02]: Base table or view not found: 1146 Table 'drupal7.simpletest748206locales_source' doesn't exist: SELECT s.lid, t.translation, s.version FROM {locales_source} s LEFT JOIN {locales_target} t ON s.lid = t.lid AND t.language = :language WHERE s.source = :source AND s.context = :context AND s.textgroup = 'default'; Array ( [:language] => de [:source] => %type: %message in %function (line %line of %file). [:context] => ) in locale() (line 674 of F:\xampplite\htdocs\drupal7\modules\locale\locale.module). Fatal error: Exception thrown without a stack frame in Unknown on line 0
Without t() , see drupal_742246.txt, I get this.
An AJAX HTTP error occurred. HTTP Result Code: 500 Debugging information follows. Path: /drupal7/batch?render=overlay&id=3&op=do StatusText: Service unavailable (with message) ResponseText: When called from JavaScript, simply output the error message. Fatal error: Exception thrown without a stack frame in Unknown on line 0
Comment #23
jbrown CreditAttribution: jbrown commentedI've replicated this. Looking into it right now.
Strangely, I can't get my interface text to translate into German.
Comment #24
jbrown CreditAttribution: jbrown commentedThis is not related to this issue.
Without the patch in this issue applied the following error still occurs:
An AJAX HTTP error occurred. HTTP Result Code: 500 Debugging information follows. Path: /drupal-7.x-dev/batch?id=8&op=do StatusText: Service unavailable (with message) ResponseText: Fatal error: Exception thrown without a stack frame in Unknown on line 0
The problem is that testing does not work if the locale module is enabled on the host installation.
Comment #25
jbrown CreditAttribution: jbrown commented@tobiasb this is the issue: #276153: Testing does not work at all when locale of host session is not English
Comment #26
jbrown CreditAttribution: jbrown commentedThe "Fatal error: Exception thrown without a stack frame in Unknown on line 0" in #22 is caused by _drupal_session_write().
We need to be catching exceptions in _drupal_session_write() in addition to _drupal_exception_handler() and _drupal_shutdown_function() as the exception handler is not and cannot be active in any of these functions.
This will assist with debugging of #276153: Testing does not work at all when locale of host session is not English
Comment #27
jbrown CreditAttribution: jbrown commentedDemoting this to normal to reduce the critical issue count.
Uncaught exceptions in _drupal_session_write() seem to be very rare.
Comment #28
rfayI officially disagree with the idea of "demoting to reduce the critical issue count".
Please demote if it does not qualify as critical, but not to make our numbers look better. Maybe that's what you're saying in your second sentence. But please, if this is critical (and I suspect it may be) leave it as critical.
http://drupal.org/node/314328:
Critical: This status is used to designate broken functionality that makes the project unusable.
Comment #29
jbrown CreditAttribution: jbrown commentedI think this issue should be normal because it really wouldn't matter very much if D7 was released without this issue resolved.
Comment #30
chx CreditAttribution: chx commentedAnd, of course, that we need ungodly ugly code
($error_level == ERROR_REPORTING_DISPLAY_ALL || (defined('MAINTENANCE_MODE') && MAINTENANCE_MODE == 'update')) {
such reflects my thoughts from #673050: Database bugs became fatals: we added exceptions and we have no strategy to really handle them. This issue IS critical and NOT just session.inc. Recently matt2000 raised the question in IRC: what happens if an exception is thrown in .install? Now that we will add fields in .install that will happen often and then you get a fatal error and then you have a broken install. And then what else do we have that we have not thought of?Once again: catch database exceptions where PDO would throw them, remove every other exception throwing and switch core to non fatal static error collections before it's too late.
Comment #31
chx CreditAttribution: chx commentedOr if someone has a better overarching strategy then come ahead with it. "exceptions are awesome" is not a strategy to handle the sudden influx of fatals. In Drupal 6 a bad query was not a fatal it was a warning.
Comment #32
dhthwy CreditAttribution: dhthwy commented+1 chx in that I think it warrants further attention. IMHO whether a database query should be fatal depends on how critical that particular query is. *I* think it should be up to the caller to decide.
Comment #33
BerdirI don't see how that ugly code (no discussion there, it *is* ugly) is related to using exceptions or not. It is just necessary to decide if we want to print an error message or not. We would need something similar if we want to properly handle errors in that function without exceptions there.
Re catching PDO exceptions and convert them to errors: This would imho totally break transaction handling as we have it now (Rollback if an exception happens during node_save() and other functions that use it), just to name an example. Also, queries are IMHO not supposed to fail, and if you really expect that they do, you can always catch the exception and handle it. I fail to see how it is bad that an broken query is fatal?
Oh, and I am very aware that our current situation is not perfect, there *are* issues with exceptions, both in PHP (that is why this issue exists) and in Drupal. But I don't think "remove all of them" is a valid solution, not at this point.
@32: And how is that not possible right now? The default simply changed from a warning to a fatal error, you can always catch the exception and handle it in any way you want.
Comment #34
chx CreditAttribution: chx commented/me shrugs. Be it. I have already won't fixed my other issue and won't repeat all that I said there. I can only hope that someone in Drupal 8 will create a proper solution for this.
Comment #35
jbrown CreditAttribution: jbrown commentedOkay - I made a pretty little function called error_displayable() .
Comment #36
chx CreditAttribution: chx commentederror_displayable surely can be written without a switch.
Comment #37
jbrown CreditAttribution: jbrown commentedLike this?
Comment #38
chx CreditAttribution: chx commentedAlmost. I do not think it worths saving a variable_get in a rarely called function like that. And so then what about
Comment #39
jbrown CreditAttribution: jbrown commentedComment #40
chx CreditAttribution: chx commentedI already RTBC'd this when it was uglier...
Comment #41
aspilicious CreditAttribution: aspilicious commentedPlease read doc style document.
Newline between @param and @return block.
Function doc has to start with 3th person verb.
Still rtbc
Comment #42
BerdirUh oh :)
There are trailing spaces...
Powered by Dreditor.
Comment #43
jbrown CreditAttribution: jbrown commentedComment #44
BerdirBetter, this was already RTBC multiple times :)
Comment #45
webchickWow, that error_displayable() wrapper function is certainly a vast improvement in terms of readability. I also asked catch to take a look, as resident benchmarker of extra function calls, and he's cool with it.
The only thing is the PHPDoc for that function needs a bit of love. For example, there is nothing there that tells me when I need to call it. And I can't quite figure out why drupal_shutdown_function and drupal_write_session use it, but other places in bootstrap do not from reading the code.
Comment #46
jbrown CreditAttribution: jbrown commentedThe only functions that need to call error_displayable() are _drupal_log_error(), _drupal_exception_handler(), _drupal_shutdown_function() and _drupal_session_write().
Ideally all errors would be reported with _drupal_log_error(), but this function is not available from _drupal_exception_handler(), _drupal_shutdown_function() and _drupal_session_write() as it may trigger an exception so these functions handle reporting themselves.
Maybe there should be a _drupal_log_error_safe() that would call error_displayable() and output whatever it is given?
I don't know what else could be added to the error_displayable() docs. It should be called when it needs to be determined whether an error should be displayed or not.
Should error_displayable() be _error_displayable() ?
Comment #47
kanzer CreditAttribution: kanzer commented#43: catch-session-exceptions.patch queued for re-testing.
Comment #48
alexanderpas CreditAttribution: alexanderpas commentededited the unholy return statement of error_displayable() a bit, no other changes.
Comment #49
Nick Lewis CreditAttribution: Nick Lewis commentedIf there was a drupal centric Seinfeld, this would have been a episode.
Comment #50
cchan CreditAttribution: cchan commentedThere is a typo in the errors.inc file. Maintenance is spelled incorrectly (missing a t) - "when in mainenance mode..."
Comment #51
rfayThis fixes the typo; no other changes.
Comment #52
jbrown CreditAttribution: jbrown commentedI'm happy with this.
Comment #53
Dries CreditAttribution: Dries commentedNice clean-up and fixes a critical problem. Committed to CVS HEAD.