[wp-trac] [WordPress Trac] #53284: `error()` method passed too many parameters

[WordPress Trac]

#53284: `error()` method passed too many parameters
--------------------------------------+-----------------------
 Reporter:  desrosj                   |       Owner:  sainthkh
     Type:  defect (bug)              |      Status:  assigned
 Priority:  normal                    |   Milestone:  5.8
Component:  Upgrade/Install           |     Version:  3.7
 Severity:  normal                    |  Resolution:
 Keywords:  good-first-bug has-patch  |     Focuses:
--------------------------------------+-----------------------
Changes (by desrosj):

 * owner:  (none) => sainthkh
 * status:  new => assigned
 * milestone:  Future Release => 5.8


Comment:

 Thanks for working on a patch, @sainthkh!

 The upgrade code unfortunately is extremely lacking in tests. In this
 case, this change could be made without adding unit tests, but we should
 just examine it extra carefully to be sure no issues will result.

 With that in mind, I took a look into what `$upgrade_result` can actually
 be set to so we could document it.

 It looks like these are the scenarios:
 - The new WordPress version when everything works.
 - `false`
 - One of several `WP_Error` objects from an inability to read or
 manipulate the filesystem from `WP_Upgrader->fs_connect()`.
 - A `WP_Error` with the `locked` error code if another update is already
 in progress.
 - A `WP_Error` with the `no_package` or `download_failed` code from
 `WP_Upgrader->download_package()`.
 - A `WP_Error` with the `incompatible_archive` code from
 `WP_Upgrader->unpack_package()`.
 - A `WP_Error` with the `copy_failed_for_update_core_file` or
 `copy_failed_space` code if the new version fails to copy to the correct
 locations.

 I think having knowledge of the error codes listed above and their
 messages could be valuable, so let's see about the best way to get both
 errors logged as feedback in the `WP_Upgrader_Skin`.

 `WP_Error` objects are able to handle multiple error codes. Calling
 `WP_Error->add()` should allow a new error to be added to the object after
 the one currently stored in `$upgrade_result`. `WP_Upgrader_Skin->error()`
 is [https://github.com/WordPress/wordpress-develop/blob/5.7/src/wp-
 admin/includes/class-wp-upgrader-skin.php#L176-L184 also able to handle
 `WP_Error` objects that contain multiple errors].

 That would allow both error messages to be logged while still only calling
 `error()` once.

 I'm assigning this ticket to you as that's the way to mark a `good-first-
 bug` ticket as claimed. I'm also moving it to the `5.8` milestone. Since
 someone picked up the ticket, it's reasonable to expect that this one can
 be wrapped up this release!

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/53284#comment:4>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform