Automattic / jetpack

Security, performance, marketing, and design tools — Jetpack is made by WordPress experts to make WP sites safer and faster, and help you grow your traffic.
https://jetpack.com/
Other
1.58k stars 799 forks source link

Instant Search: handle API failures and displaying failure info #13366

Open gibrown opened 5 years ago

gibrown commented 5 years ago

We should handle the API errors based on whether we got a response and what the error code is. Any 4xx should not be run again. 5xx should probably be retried after a bit of time.

I am not sure what we should display in the UI. Depends a lot on if there is already data getting displayed.

bluefuton commented 4 years ago

@jsnmoon I know we implemented a fallback to cached results - did we do anything about error handling?

jsnmoon commented 4 years ago

@bluefuton We added network error handling to treat all non-ok responses as "offline". We could use more polish on this here:

https://github.com/Automattic/jetpack/blob/instant-search-master/modules/search/instant-search/components/search-results.jsx#L69

stale[bot] commented 4 years ago

This issue has been marked as stale. This happened because:

No further action is needed. But it's worth checking if this ticket has clear reproduction steps and it is still reproducible. Feel free to close this issue if you think it's not valid anymore — if you do, please add a brief explanation.

gibrown commented 4 years ago

Some use cases:

Maybe we should just display the returned error message for all 400 and 500 http errors? This could also aid in debugging anytime the end user writes some custom filters.

gibrown commented 3 years ago

Based on some of the experiences with private sites (e.g. a broken sandbox), it looks like our error handling messages may not be fully working in all cases.