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 code provided in system.api.php includes an UpdateException on line 2797 within the hook_update_N function. The statement immediately before that line is a return statement which will ensure that the exception will never be thrown.
Either remove the Exception or reorder the operations so that the Exception is before the return statement.
Comment | File | Size | Author |
---|---|---|---|
#18 | 1788610-hook_update_N_exception-14.patch | 981 bytes | _12345678912345678 |
#13 | 1788610-hook_update_N_exception-13.patch | 981 bytes | _12345678912345678 |
#11 | 1788610-hook_update_N_exception-11.patch | 983 bytes | fjd |
#7 | 1788610-hook_update_N_exception.patch | 953 bytes | chertzog |
#5 | 1788610-hook_update_N_exception.patch | 954 bytes | chertzog |
Comments
Comment #1
larowlanTagging
Comment #2
BrockBoland CreditAttribution: BrockBoland commentedSince this is just sample code that never actually runs, it's probably not a big deal that it's kind of weird, but it certainly could be a better example.
I think that the example would be more helpful if the exception throw were inside of a conditional—something like, if this function call fails, throw an exception. At the moment, I can't think of a good example of what might be checked, though.
Comment #3
BrockBoland CreditAttribution: BrockBoland commentedAlso, just for convenience, this code can be seen in the docs here:
http://api.drupal.org/api/drupal/modules%21system%21system.api.php/funct...
Comment #4
nick_schuch CreditAttribution: nick_schuch commentedUpdating assignee.
Comment #5
chertzogHere is a patch that moves the exception to before the return and puts in a check that would never pass.
Comment #6
fjd CreditAttribution: fjd commentedPatch applied cleanly. Verified the exception code now appears before the return statement.
I noticed the conditional does not have the standard formatting, there is no white space between the 'if' and opening brackets.
I'm not sure how to test for the thrown exception. Will the statement ever pass and throw the exception? If the exception is never thrown, is there any need for it?
Comment #7
chertzogIts in the API docs, so the code never runs. The exception is just there to demonstrate usage. That conditional should never pass.
Comment #8
fjd CreditAttribution: fjd commentedPatch applied cleanly. Verified the condition and exception appear before the return statement. Condition is formatted correctly.
Comment #9
markpavlitski CreditAttribution: markpavlitski commentedAgreed, patch looks good.
Comment #10
alexpottThe check here looks spurious... some like this is better...
Comment #11
fjd CreditAttribution: fjd commentedCreated patch based on feedback in comment #10.
Comment #12
star-szrLooks good other than trailing whitespace, please remove.
Comment #13
_12345678912345678 CreditAttribution: _12345678912345678 commentedAs part of drupalcamp nh codesprint I rerolled this patch with spaces taken out.
Comment #14
galooph CreditAttribution: galooph commentedPatch applied cleanly, verified that the trailing whitespace mentioned in #12 has been removed.
Comment #15
_12345678912345678 CreditAttribution: _12345678912345678 commentedThanks for checking :)
Comment #16
star-szrThanks, looks good! An interdiff is also very helpful to create and upload when updating patches, so you know for next time.
Comment #17
alexpottCommitted 9ba259b and pushed to 8.x. Thanks!
During commit set file permissions back to 644 - the patches in #11 & #13 changed them 755
Comment #18
_12345678912345678 CreditAttribution: _12345678912345678 commentedI fixed the permissions hopefully this is fine let me know if it needs any other changes. Thanks