Allow theme and plugin list generation to fail gracefully

[wojtekn]

Related issues

• Fixes https://github.com/Automattic/dotcom-forge/issues/9582

Proposed Changes

I propose to return an empty array when Studio gets an error from WP CLI when it tries to get a list of plugins or themes for export purposes.

Testing Instructions

1. Export site from Studio
2. Open package
3. Locate and open meta.json file
4. Confirm that file includes list of themes and plugins
5. Confirm that regression test passes

Pre-merge Checklist

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

[nightnei]

Is see in DoD of the task Application doesn't crash and send error to Sentry when WP CLI can't get plugins or themes details, but according to the code, looks like we just do console.error, so I wonder - will it be definitely logged to Sentry, somehow under the hood? What disturbs me is it looks like we are muting errors now. Or maybe I am missing something.

Also, I am thinking - is it expected to have empty array? What is we have some plugins/theme, looks like they will be ignored, won't they?

[wojtekn]

Is see in DoD of the task Application doesn't crash and send error to Sentry when WP CLI can't get plugins or themes details, but according to the code, looks like we just do console.error, so I wonder - will it be definitely logged to Sentry, somehow under the hood? What disturbs me is it looks like we are muting errors now. Or maybe I am missing something.

console.error() was not the code that was responsible for triggering the error sent to Sentry. Without that return, code execution continued and resulted in throwing a parse error in return JSON.parse( stdout );.

Also, I am thinking - is it expected to have empty array? What is we have some plugins/theme, looks like they will be ignored, won't they?

Yes, in such a case, an entry in meta.json would be empty. It's not great, but without that, the exported package will still be usable in most cases.

[nightnei]

console.error() was not the code that was responsible for triggering the error sent to Sentry. Without that return, code execution continued and resulted in throwing a parse error in return JSON.parse( stdout );.

I mean, here, we have "Done is: Application doesn't crash and send error to Sentry when WP CLI can't get plugins or themes details".
But with current changes we don't "send error to Sentry when WP CLI can't get plugins or themes details".
So looks like we just mute errors and won't get them in Sentry. Should we manually send them without crashing the flow?

Yes, in such a case, an entry in meta.json would be empty. It's not great, but without that, the exported package will still be usable in most cases.

I am not enough in context, but I have a personal feeling that it's better to show the errors instead of allowing using a site that doesn't have all the original data. Since it can make frustration for users, with feeling like "There should be plugins/theme, but I don't have them and I didn't know why, since I didn't have errors".
Do we know the reason for crashing and can we handle it somehow? It's inside Studio, so looks like we have all the control.

[wojtekn]

Application doesn't crash and send error to Sentry when WP CLI can't get plugins or themes details".

And I think it exactly does that now: it doesn't crash and doesn't send errors to Sentry.

I am not enough in context, but I have a personal feeling that it's better to show the errors instead of allowing using a site that doesn't have all the original data. Since it can make frustration for users, with feeling like "There should be plugins/theme, but I don't have them and I didn't know why, since I didn't have errors".

The exported package will include all plugins and themes inside wp-content/. It won't include entries for them in meta.json if the WP CLI command fails, though.

Do we know the reason for crashing and can we handle it somehow? It's inside Studio, so looks like we have all the control.

I think that a potential case is when a user installed a plugin that results in dumping some HTML when the wp cli command is executed.

[nightnei]

And I think it exactly does that now: it doesn't crash and doesn't send errors to Sentry.

Oh, I see 😅 When I was reading, I applied "doesn't" only for the first part of the sentence :)

Thanks for the answers, LGTM 👍