[wp-trac] [WordPress Trac] #52892: Privacy Export Personal Data: JSON encoding failure generates invalid JSON in export file

[WordPress Trac]

#52892: Privacy Export Personal Data: JSON encoding failure generates invalid JSON
in export file
---------------------------------------------+-----------------------------
 Reporter:  hellofromTonya                   |       Owner:  hellofromTonya
     Type:  defect (bug)                     |      Status:  assigned
 Priority:  normal                           |   Milestone:  5.8
Component:  Privacy                          |     Version:  5.4
 Severity:  normal                           |  Resolution:
 Keywords:  has-patch has-unit-tests commit  |     Focuses:
---------------------------------------------+-----------------------------

Comment (by SergeyBiryukov):

 Thanks for the PR!

 * It's not quite clear to me why we only append `json_last_error_msg()` if
 there is no error code. In my testing, that leads to not getting any
 additional information about the error, while the actual error in my case
 was "Malformed UTF-8 characters, possibly incorrectly encoded". If
 `json_last_error()` was `JSON_ERROR_NONE` or empty, I don't think
 `json_encode()` would return `false` in the first place, unless I'm
 missing something. So I think it would make sense to always include
 `json_last_error_msg()` here. Something like:
 {{{
 $error_message = sprintf(
         /* translators: %s: Error message. */
         __( 'Unable to encode the personal data for export. Error: %s' ),
         json_last_error_msg()
 );
 }}}
 * Adding a `remove_filter()` call to the `::tearDown()` method is not
 needed here. I understand that from a consistency point of view it seems
 like that if we add a filter somewhere, we should remove it as well,
 however the base `WP_UnitTestCase_Base::tearDown()` method already handles
 that for us via `::_restore_hooks()`, see [29251] / #28535 for more
 details. So removing the filter ourselves is redundant, similar instances
 were previously removed in [50463] / #52625.

 [attachment:52892.diff] includes these suggestions.

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