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.
Now that we have _drupal_log_error(), we should ensure it plays nice with BatchAPI so that errors are displayed in the result.
Comments
Comment #1
webchickCopying description from #363911: Batch API needs to return useful error messages, since this issue didn't come up in my search results.
This issue came up while testing #363262: Missing index on url_alias table, but works for really any Batch API-related thing. When Batch API encounters an error, instead of something actually helpful such as "I'm sorry, that index already exists," it gives you the completely unhelpful:
An easy way to replicate this is to rename system_update_7018 it to system_update_7999 and run it again.
This is getting masked right now because we removed the ability to select past updates from update.php. It also happens in SimpleTest from time to time. Needs to be fixed before release or the support requests will be unbearable.
Comment #2
Damien Tournoud CreditAttribution: Damien Tournoud commentedHere is a first shot at this. The security impact of allowing HTML messages to be displayed should be evaluated.
Comment #4
Damien Tournoud CreditAttribution: Damien Tournoud commentedRerolled, and fixed the test ;)
Comment #6
Damien Tournoud CreditAttribution: Damien Tournoud commentedLet's try again.
Comment #8
Damien Tournoud CreditAttribution: Damien Tournoud commentedComment #10
Damien Tournoud CreditAttribution: Damien Tournoud commentedAnd an even better version.
Comment #11
burningdog CreditAttribution: burningdog commentedThis works for me. Batch API wasn't handling a PDOException and was simply returning a "An HTTP error 500 occurred" error. Applying Damien's patch resulted in a useful error message (about the PDOException) being returned to the browser.
Comment #12
sunPlease use "JavaScript" here.
Given the complexity of the following condition, "proper" sounds like something no one should understand. Can we change this, so developers understand?
Comment #13
Damien Tournoud CreditAttribution: Damien Tournoud commentedRerolled!
Error reporting levels are now defined using those three constants:
Comment #15
Damien Tournoud CreditAttribution: Damien Tournoud commentedOups. I messed-up with the constants a bit. This one should work.
Comment #16
sunHm. Don't we have these already?
IMO, it does not make sense to introduce yet another set of severity levels. Their intention and purpose are the same, just the target (watchdog/dsm) differs.
Comment #17
Damien Tournoud CreditAttribution: Damien Tournoud commented@sun: we are not introducting anything, those were already there, I'm only giving them names ;)
Comment #18
sunok, agreed. Should be reworked in a separate issue.
That, I don't understand. The old code was correct, so please revert. ;)
After that fix, mark as RTBC, please.
Comment #19
Damien Tournoud CreditAttribution: Damien Tournoud commentedDone. It was a merging artifact.
Marking RTBC as requested by sun.
Comment #20
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.