Add try...catch to handle backup API failure

[renintw]

Fixes #182

This PR adds try...catch to handle backup API failure, rendering an error notice when the API doesn't work correctly.

Testing Steps

Go to account-status.js and comment out disabled={ ! backupCodesEnabled } on L78.

Scenario 1 - error.code === 'revalidation_required'

1. Go to wp-content/plugins/two-factor/class-two-factor-core.php and remove the ! for the if condition on L1209.
   => if ( self::current_user_can_update_two_factor_options( 'save' ) ) {
2. Go to /?screen=backup-codes and it's expected to have the same results as the screenshot below.

Local

Sandbox

Scenario 2 - other kinds of error

1. Go to wp-content/plugins/two-factor/providers/class-two-factor-backup-codes.php and make it always throw an error on L295.
2. Go to /?screen=backup-codes and it's expected to have the same results as the screenshot below - an error notice with an error message is shown.

Question

Do we want to show error messages dynamically on the front-end side, or alternatively, a certain error message like "Something went wrong, please seek assistance somewhere" could probably be displayed? As the error message like "Unable to enable recovery codes for this user." needs rephrasing a bit.

[iandunn]

show error messages dynamically ... or a certain error message like "Something went wrong, please seek assistance somewhere"

IMO it's best to show a specific message, because that helps us troubleshoot when someone reports a bug. Otherwise it can be harder to reproduce. I don't feel strongly, though.

[iandunn]

Looks good overall 👍🏻

I think it's worth considering how revalidation is handled globally, but don't object to merging this either.

[iandunn]

🤔 I feel like it might get hard to maintain if every component has to be aware of revalidation, and build in logic to handle it. I wonder if we can do something that'd covers all components, without them having to be coupled to it? That's probably outside the scope of this PR, though.

I'd lean towards removing the revalidation stuff here, and just having the Notice to catch any kind of API error. I don't feel strongly, though.

If you decide to keep the revalidation, then I think it'd be good to return the user to the Backup Codes screen after they revalidate, so they can pick up where they left off. Otherwise I worry the flow might feel disjointed to them.

[StevenDufresne]

Agreed. I would lean towards using an apiFetch middleware: https://www.npmjs.com/package/@wordpress/api-fetch.

[renintw]

Tried using apiFetch middleware, but it didn't go well. Context couldn't be accessed in the middleware and the RevalidateModal needs it. I've tried several other ways and found out that one seems to work with a quick test. I'll open another PR for it after finishing the rest part of it. And for this PR, I'll remove the revalidation stuff here, and just have the Notice to catch any kind of API error. Thanks for bringing this up, it's a really good point. And thanks Steve for sharing a potential solution! 👍

[StevenDufresne]

So I understand it correctly, you wanted to set the error within the app context and you couldn't?

[renintw]

This is the branch I tried with apiFetch middleware, it's in the status that can be tested directly. I was trying to add a specific handler for revalidation using middleware, making it render the components we want if 'revalidation_required' !== error.code. However, as createRoot or any other way that dynamically renders components would build a new tree that's unable to access the Context data, it would lead to an error message. This would work if only the component Notice is rendered.

I also tried setting variables or flagging when the error occurs in the first place, and then having components detect them to trigger the re-rendering accordingly. However, this PR might be more straightforward.

[pkevan]

IMO it's best to show a specific message, because that helps us troubleshoot when someone reports a bug.

Agreed - always easier to have a specific place to start debugging from, rather than needing to reproduce :)