Studio: Output proper message on push error

[ivan-ottinger]

Related issues

• Related to https://github.com/Automattic/dotcom-forge/issues/10009.

Proposed Changes

• output relevant message on push error

Testing Instructions

1. Apply the backend diff (D167588-code) to your sandbox and sandbox public-api.wordpress.com.
2. Check out this PR and build the app with npm start.
3. Navigate to the Sync tab and make sure you have a WPCOM site with its staging site connected.
4. While at the staging site settings tab (https://wordpress.com/staging-site/{site-slug}), initiate a sync from staging to production.
5. Head back to the Sync tab in Studio and try to initiate a push operation.
6. After some time, a dialog with relevant error message should display (as can be seen in the screenshot above).

Pre-merge Checklist

• Have you checked for TypeScript, React or other console errors?

[fredrikekelund]

Works as expected 👍 However, I think we should make a small tweak to the code. I'll still approve straight away, though, to speed things up

[fredrikekelund]

Suggested change

	if ( typeof error === 'object' && error !== null && 'error' in error ) {
	errorMessage = ( error as { error: string } ).error || errorMessage;
	}
	if ( typeof error?.error === 'string' && error.error ) {
	errorMessage = error.error;
	}

I did some digging to try and find a better way to identify if error is of the right type. I couldn't find anything great, but I still think we can improve the current solution slightly.

[ivan-ottinger]

Thank you for your suggestion, @fredrikekelund. Originally, I had a simpler check in place, but TS was not happy about it. Even though I prefer the suggested change, it still triggers the following error:

Property 'error' does not exist on type '{}'.

Every alternative I have tried so far is quite verbose too. 🤔

[wojtekn]

Would it make sense to move this logic to a separate function so that instead of redefining the variables, we could rely on early returns?

[fredrikekelund]

TS was not happy about it

Seems like I overlooked the TS errors in my suggestion yesterday…

move this logic to a separate function

Yeah, adding a getErrorMessageFromResponse function, or alternatively, an isDecoratedErrorResponse function seems like a good idea. It would help clean up this code. Those function names are obviously just suggestions.

[ivan-ottinger]

Thank you, Wojtek and Fredrik. I have extracted out the logic in the latest commit 9439ade. Feel free to let me know what you think.

[ivan-ottinger]

Just to note that I have now simplified the logic even further based on Wojtek's suggestion.