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.When PHP dies because of a run-time error in an update function (undefined function, whatever), the AHAH call returns some valid HTML like
<br />
<b>Fatal error</b>: Call to undefined function foo_bar() in <b>foo.inc</b> on line <b>nnn</b><br />
and the progressbar component breaks with the message "An HTTP error 200 occurred.", which is absurd, 200 being 'OK'...
Attached patch fixes this, displaying 'an error occurred' instead
| Comment | File | Size | Author |
|---|---|---|---|
| #20 | install_profile_error.png | 22.6 KB | asimmonds |
| #19 | HTTP_error_200.patch | 3.54 KB | yched |
| #9 | HTTP_error_200.patch | 1.18 KB | yched |
| #7 | HTTP_error_200.patch | 953 bytes | yched |
| #5 | HTTP_error_200.patch | 827 bytes | yched |
Comments
Comment #1
yched CreditAttribution: yched commentedBetter : if the server has error display on, we might as well display the error message...
Comment #2
moshe weitzman CreditAttribution: moshe weitzman commentedThis bug makes it extremely hard to recover from an unsuccessful update.
Comment #3
moshe weitzman CreditAttribution: moshe weitzman commentedtested patch. it gets rid of the silly 200 error but for me it provided no helpful info. the error i get is:
An error occurred. http://www.tejasa.com/update.php?id=7&op=do
Comment #4
Gábor HojtsyLooks like Moshe does not get the responseText. Also, coding style looks mixed: we should not have a space before the comma and the :
Comment #5
yched CreditAttribution: yched commentedI'd say moshe gets a WSOD :-)
If your server is set not to display any error message, then the response text is plain empty and you get no hint on what error occurs, just like for any other non-AHAH WSOD. Not much we can do about that, I'm afraid.
Attached patch fixes spacing style errors.
Comment #6
Gábor Hojtsyyched: well the WSOD reason might be it, yes.
Sorry for not noting this before, but as with t() calls in PHP, this one also need the text itself wrapped in Drupal.t(), not some variable, or the text will not be extracted. We don't have a JS runtime at hand to get what might be the value of the message variable when we extract the translatable strings. So wrap the text itself in Drupal.t(). Sorry that I did not catch this above.
Otherwise looks good to me.
Comment #7
yched CreditAttribution: yched commentedBleh. Of course, I should know that...
Comment #8
Gábor Hojtsy- Any reason, why we have a space after the dots at end of sentences?
- What about adding a Drupal.t('No information provided about the error by the server.') or something along these lines when we have no responseText?
Comment #9
yched CreditAttribution: yched commentedAdditional 'no information' text when responseText empty : why not - not exactly sure how we detect WSOD's, though, How about this ?
Space after the dot : dunno, the message in current HEAD has this, I left it as is. Error message in autocomplete.js has this as well, BTW - probably as a result of copy-paste... Which makes me wonder : do we want to fix 'HTTP error 200' in autocomplete too... ? Do we need to abstract this in a drupal.js function, then ?
Comment #10
Gábor Hojtsy- I am not exactly sure about this string check, you can surely do ".length == 0" on the string, but it is possible that casting works to boolean this way. I am not much of a JS magician :)
- Good catch. Indeed, good idea to fix that as well with abstraction. This looks like a quite generic API function.
Comment #11
moshe weitzman CreditAttribution: moshe weitzman commentedI finally switched to non js mode and saw that my error was memory related - see below. The error is in system update 6029, BTW. That one does a menu rebuild().
Fatal error: Allowed memory size of 16777216 bytes exhausted (tried to allocate 192507 bytes) in /home/weitzman/public_html/tejasa.com/includes/cache.inc on line 104
Comment #12
Gábor HojtsyMoshe: Hm, interesting. That error should have been shown on the JS version too.
Comment #13
yched CreditAttribution: yched commented@moshe : So using non JS mode, you get the PHP error ? where ? inside a nicely themed maintenance page, or as a raw error page ?
(edit : fix typo)
Comment #14
chx CreditAttribution: chx commentedI wonder whether http://drupal.org/node/202955 this solves the menu rebuild memory issues.
Comment #15
moshe weitzman CreditAttribution: moshe weitzman commentedthe error was toward the bottom of a nicely themed page
Comment #16
neofactor CreditAttribution: neofactor commentedThis is the error I get during INSTALL of 6.0rc1:
An HTTP 200 occurred.
http://localhost/drupal-6.0-rc1/install.php?locale=en&profile-default&id...
It provides a link to "the error page" but it is blank.
This happens everytime for me on local MAMP install. It does not happen on my live server.
Thoughts?
Comment #17
Gábor Hojtsyneofactor: did the above patch provide the error message properly for you?
Comment #18
yched CreditAttribution: yched commentedNew patch factors out the error message building in Drupal.ahahError(), and lets autocomplete.js and progress.js use it. ahah.js currently doesn't have any error handling, maybe this should go in there too.
@moshe : I can't seem to reproduce. I mean, if I set the memory_limit low enough, I do get the 'Allowed memory size exhausted' error when running a D5-D6 upgrade (as expected), but the error message is displayed even in JS mode.
Can you provide more info on your setup ? (display_errors and error_reporting settings, w or w/o xdebug...)
Also, could you use firebug or smthg similar to report what's the last XHR response you receive ?
@neofactor : It provides a link to "the error page" but it is blank.
does it mean the error page itself is a white screen ? If so, it could mean that the generation of that page gets hit by memory limit as well...
This thread is not about *fixing* memory errors, but about providing proper error notification, but it would be nice if you could try the patch and see if you do get an explicit error message.
Comment #19
yched CreditAttribution: yched commentedahah.js currently doesn't have any error handling
I was wrong, it's just slightly different since it performs the ajax call through jquery_form.js's ajaxSubmit().
Updated patch uses the new Drupal.ahahEror() to generate ahah.js error messages. Also fixes an undefined 'element' variable in ahah.js error handler.
That specific part can be tested by adding a foobarbaz(); call at the top of poll.module's poll_choice_js(), and hitting the 'add another choice' button on a poll node edit form.
(edit : actually attach the patch...)
Comment #20
asimmonds CreditAttribution: asimmonds commentedThe attached screenshot is what I get when I run a fresh install after doing something stupid to the install profile, with patch from #19 applied.
Is it possible to remove the <br /> tag from the error message, which looks kind of silly?
The install now stops, which is much better than before, but the only other significant problem for the installer is that the 'the error page' link goes to the 'configure site' page (http://localhost/drupal-cvs/install.php?locale=&profile=sandboxall&id=1&...)
Looking at the installer code, it looks to me like the installer doesn't handle any batch errors, which is probably unrelated to this issue.
Comment #21
yched CreditAttribution: yched commented@asimmonds : the screenshot shows the expected behavior, that's cool.
I noticed the
<br />too, of course, but I can't seem to understand why it's not parsed. It's in the responseText, just like the<b>tags, and those get parsed OK. As a last resort, we could manually regexp it out, but that sounds awkward...About the 'no error page for failed install batch' : that's true actually, I must have overlooked that in http://drupal.org/node/176003. Actually the pre-existing 'locale import' batches did not have any either, but I guess much less stuff can go wrong in local imports than in modules install...
This should probably be dealt with in a separate issue, though. I opened http://drupal.org/node/205269 for this.
Comment #22
Gábor HojtsyExcept that I don't see why "$(this.element)." needed to be used instead of "element.", the patch looks good.
Comment #23
yched CreditAttribution: yched commentedI don't see why "$(this.element)." needed to be used instead of "element."
This is a side-kick bugfix : 'element' is undefined, it's this.element that exists, and it's a DOM element, not a jQuery object, son it needs to be $()ed if we want its
.parent().That's the "Also fixes an undefined 'element' variable in ahah.js error handler." in comment #19 holding the last patch.
Comment #24
Gábor HojtsySince we actually only have Moshe experiencing some problems with the patch, everyone else tested sees the error messages fine, so I went ahead and committed this patch. I am not on the opinion that we need to hardcode a certain number of tags removed (like the
<br />tag). Let's have the error messages intact, not risking modifying something which should actually be there in the message; all better for error reporting in the forums.Comment #25
(not verified) CreditAttribution: commentedAutomatically closed -- issue fixed for two weeks with no activity.