Skip to content
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

Show an error message when Launchpad fails to load data #45

Merged
merged 1 commit into from
Jun 5, 2024

Conversation

jdgarcia
Copy link
Contributor

@jdgarcia jdgarcia commented Jun 5, 2024

https://gitkraken.atlassian.net/browse/GKCS-5849

There was no design for this so I just made one up. Let me know if you have any design suggestions.

Cloud-hosted:
image

Self-hosted:
image

Testing Notes:
You may want to add retry: 0 to the queryClient default options since by default react-query retries 3 times with exponential backoff, so it can take a while for the error message to show up.

Copy link
Contributor

@gitkrakel gitkrakel left a comment

Choose a reason for hiding this comment

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

LGTM 👍 I think the design is fine given the error probably won't be seen unless the popup is open for a long time. Our hands are tied here because of what the provider-api package provides (and also the Bitbucket proxy).

Just thinking out loud, if provider-api package gave us more info, we could give better messaging to users about why it's taking so long to load data:

  • On first retry, we display a message "Re-attempting to fetch provider data"
  • On second retry, we display a message "Still failing to fetch data. Is your access token still valid?" (if the error status is 401)
  • After all retries are exhausted, we can display a more specific error based on the status code we get back.

That way the user won't feel like the extension is doing nothing for a long time. Anyway, food for thought for future PRs.

Comment on lines +207 to +211
Make sure the service at{' '}
<ExternalLink className="text-link" href={error.domain}>
{error.domain}
</ExternalLink>{' '}
is reachable. If it is, make sure your access token is still valid.
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO this message is a bit long. Normally I'd want to split it into two cases, one is the domain isn't reachable, and another is the access token is bad.

However I see that we can't distinguish that with the errors thrown by the provider-api package, so this is fine as-is.

@jdgarcia jdgarcia merged commit 1d4df71 into main Jun 5, 2024
1 check passed
@jdgarcia jdgarcia deleted the launchpad-error-message branch June 5, 2024 23:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants