Display specific error message if fail to retrieve permissions caused by rate limit #8658
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PopDaph
force-pushed
the
failed-permissions-rate-limit
branch
from
deee4b3 to
cf38d98
Compare
tdraier
approved these changes
Nov 15, 2024
front/lib/api/data_source_view.ts
Outdated
Comment on lines
112
to
119
| if (connectorsRes.error.type === "connector_rate_limit_error") { | ||
| return new Err( | ||
| new Error( | ||
| connectorsRes.error.message ?? | ||
| "An error occurred while fetching the resources' content nodes." | ||
| ) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Why not doing this for any error ? connectorsRes.error.message will usually provide more useful information ?
There was a problem hiding this comment.
I asked myself the question and decided to not do it because they are usually not user friendly messages and not meant to be user facing. I simplified the code pointed here though!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This is a change on the Slack connector. Currently, when there's an error while trying to retrieve permission (which we use on the permission modal + on the modal to set an assistant as default to answer in a Slack channel), we always display the wording "Failed to retrieve permissions likely due to a revoked authorization.", which is not necessarily true, the error can be something else such as hitting a rate limit from Slack: The goal of this PR is to display a proper message on these screens when the error we hit is a rate limit from Slack.
To do so, in connectors, in case of a rate limit:
SlackManager.retrievePermissions()can return aProviderWorkflowError."connector_rate_limit_error"which is an existing error code in connector.In front:
"connector_rate_limit_error"we answer a"rate_limit_error"which is a frontAPI error type.useConnectorPermissionsto also return the error (not just a boolean to know if there's an error).UseResourcesHookto have this optional new param with the error.ContentNodeTreeChildren, if there's an error and it's a rate limit error, we display a proper message.This is not fixing the why we get a rate limit, but this error message is much clearer and less scary for users (their token is not revoked, we are still syncing their data).
Risk
Can be rolled back but since it's both front & connector it's annoying.
Deploy Plan