Skip to content

Reduce error boundaries for connections, sources, destinations and onboarding, add retry button. #12848

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
May 24, 2022

Conversation

edmundito
Copy link
Contributor

@edmundito edmundito commented May 13, 2022

What

This improves how we handle errors in the app. Previously, any network error would cause the app to go into a blank screen with an error. Now the navigation bar will remain present. Additionally, it enables the user to retry the request.

Related to #12172

Onboarding error:

Screen Shot 2022-05-13 at 15 04 33

Source/Connection/Destination list error:
Screen Shot 2022-05-13 at 15 05 46

Source/destination settings error:
Screen Shot 2022-05-13 at 15 10 21

How

  • Updates the ApiErrorBoundary component with retry functionality react-query useQueryResetErrorBoundary. Resets on location change.
  • Adds the ApiErrorBoundary to the main routes so that it doesn't error outside of the navigation
  • Adds the ApiErrorBoundary component to onboarding, source, destination, and connection components so that it doesn't error the entire page

Recommended reading order

Top to bottom

@github-actions github-actions bot added area/frontend area/platform issues related to the platform labels May 13, 2022
@edmundito edmundito force-pushed the edmundito/reduce-error-boundary branch from 0c1607f to 2bef0cc Compare May 18, 2022 15:16
@edmundito edmundito marked this pull request as ready for review May 18, 2022 15:20
@edmundito edmundito requested a review from a team as a code owner May 18, 2022 15:20
@teallarson
Copy link
Contributor

Resets on location change.

I wonder about a few spots in the code where we change "steps" but it isn't reflected in the route (ie: Connection workflow and Onboarding workflow). Would that cause any buggy/unexpected behavior with this? I don't think it would and seems like either way it's an improvement over previous behavior. I'm just curious.

@edmundito edmundito force-pushed the edmundito/reduce-error-boundary branch from 2bef0cc to d637b83 Compare May 23, 2022 17:35
Copy link
Contributor

@timroes timroes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested this locally and blocked some requests via the browser dev tools. Seems to work as expected on the pages I tested. Code LGTM

@edmundito edmundito merged commit 3b93dd8 into master May 24, 2022
@edmundito edmundito deleted the edmundito/reduce-error-boundary branch May 24, 2022 19:06
jscottpolevault pushed a commit to jscottpolevault/airbyte that referenced this pull request Jun 1, 2022
…boarding, add retry button. (airbytehq#12848)

* Add retry logic to ApiErrorBoundary component and move boundary one level deeper

* Simplify cannot reach message error message on cloud.

* Update ApiErrorBoundary to use Button component, add strings to en.json

* Add clearOnLocationChange flag to ApiErrorBoundary to clear the error when navigating to another page

* Add error boundaries to DestinationItemPage and SourceItemPage

* Update ApiErrorBoundary to include ability to reset enabled by optional prop withRetry

* Remove location and retry options from ApiErrorBoundary

* Add ApiErrorBoundary to onboarding page

* Show titles on most onboarding steps even in error

* Add the ability to hide the header for ApiErrorBoundaries and BaseClearView

* Remove retry logic from state change in ApiErrorBoundary
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/platform issues related to the platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants