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

Display specific error message if fail to retrieve permissions caused by rate limit #8658

Merged
merged 4 commits into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions connectors/src/api/get_connector_permissions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type { Request, Response } from "express";

import { getConnectorManager } from "@connectors/connectors";
import { augmentContentNodesWithParentIds } from "@connectors/lib/api/content_nodes";
import { ProviderWorkflowError } from "@connectors/lib/error";
import logger from "@connectors/logger/logger";
import { apiError, withLogging } from "@connectors/logger/withlogging";
import { ConnectorResource } from "@connectors/resources/connector_resource";
Expand Down Expand Up @@ -85,6 +86,18 @@ const _getConnectorPermissions = async (
});

if (pRes.isErr()) {
if (
pRes.error instanceof ProviderWorkflowError &&
pRes.error.type === "rate_limit_error"
) {
return apiError(req, res, {
status_code: 429,
api_error: {
type: "connector_rate_limit_error",
message: pRes.error.message,
},
});
}
return apiError(req, res, {
status_code: 500,
api_error: {
Expand Down
12 changes: 11 additions & 1 deletion connectors/src/connectors/slack/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ import {
getSlackClient,
} from "@connectors/connectors/slack/lib/slack_client";
import { launchSlackSyncWorkflow } from "@connectors/connectors/slack/temporal/client.js";
import { ExternalOAuthTokenError } from "@connectors/lib/error";
import {
ExternalOAuthTokenError,
ProviderWorkflowError,
} from "@connectors/lib/error";
import { SlackChannel } from "@connectors/lib/models/slack";
import { terminateAllWorkflowsForConnectorId } from "@connectors/lib/temporal";
import logger from "@connectors/logger/logger";
Expand Down Expand Up @@ -404,6 +407,13 @@ export class SlackConnectorManager extends BaseConnectorManager<SlackConfigurati
new Error("Slack token invalid. Please re-authorize Slack.")
);
}
if (e instanceof ProviderWorkflowError && e.type === "rate_limit_error") {
logger.error(
{ connectorId: this.connectorId, error: e },
"Slack rate limit when retrieving permissions."
);
return new Err(e);
}
throw e;
}
}
Expand Down
5 changes: 1 addition & 4 deletions connectors/src/connectors/slack/lib/slack_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,7 @@ export async function getSlackClient(
// If we get rate limited, we throw a known error.
// Note: a previous version using slackError.code === ErrorCode.RateLimitedError failed
// see PR #2689 for details
if (
e instanceof Error &&
e.message.startsWith("A rate limit was exceeded")
) {
if (e instanceof Error && e.message.includes("rate limit")) {
try {
Context.current().heartbeat();
await Context.current().sleep("1 minute");
Expand Down
14 changes: 11 additions & 3 deletions front/components/ContentNodeTree.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
Tooltip,
Tree,
} from "@dust-tt/sparkle";
import type { BaseContentNode } from "@dust-tt/types";
import type { APIError, BaseContentNode } from "@dust-tt/types";
import type { ReactNode } from "react";
import React, { useCallback, useContext, useState } from "react";

Expand All @@ -35,6 +35,7 @@ export type UseResourcesHook = (parentId: string | null) => {
resources: BaseContentNode[];
isResourcesLoading: boolean;
isResourcesError: boolean;
resourcesError?: APIError | null;
};

export type ContentNodeTreeItemStatus = {
Expand Down Expand Up @@ -112,7 +113,7 @@ function ContentNodeTreeChildren({

const { useResourcesHook, emptyComponent } = useContentNodeTreeContext();

const { resources, isResourcesLoading, isResourcesError } =
const { resources, isResourcesLoading, isResourcesError, resourcesError } =
useResourcesHook(parentId);

const filteredNodes = resources.filter(
Expand Down Expand Up @@ -152,7 +153,14 @@ function ContentNodeTreeChildren({
if (isResourcesError) {
return (
<div className="text-sm text-warning">
Failed to retrieve permissions likely due to a revoked authorization.
{resourcesError?.type === "rate_limit_error" ? (
<>Connected service's API limit reached. Please retry shortly.</>
) : (
<>
Failed to retrieve permissions likely due to a revoked
authorization.
</>
)}
</div>
);
}
Expand Down
3 changes: 3 additions & 0 deletions front/lib/api/data_source_view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ async function getContentNodesForManagedDataSourceView(
});

if (connectorsRes.isErr()) {
if (connectorsRes.error.type === "connector_rate_limit_error") {
return new Err(new Error(connectorsRes.error.message));
}
return new Err(
new Error(
"An error occurred while fetching the resources' children content nodes."
Expand Down
5 changes: 4 additions & 1 deletion front/lib/swr/connectors.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { useSendNotification } from "@dust-tt/sparkle";
import type {
APIError,
ConnectorPermission,
ContentNodesViewType,
DataSourceType,
Expand All @@ -21,7 +22,8 @@ import type { GetDataSourcePermissionsResponseBody } from "@app/pages/api/w/[wId
interface UseConnectorPermissionsReturn<IncludeParents extends boolean> {
resources: GetDataSourcePermissionsResponseBody<IncludeParents>["resources"];
isResourcesLoading: boolean;
isResourcesError: any;
isResourcesError: boolean;
resourcesError: APIError | null;
}

export function useConnectorPermissions<IncludeParents extends boolean>({
Expand Down Expand Up @@ -77,6 +79,7 @@ export function useConnectorPermissions<IncludeParents extends boolean>({
),
isResourcesLoading: !error && !data,
isResourcesError: error,
resourcesError: error ? (error.error as APIError) : null,
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,16 @@ export async function getManagedDataSourcePermissionsHandler(
includeParents,
});
if (permissionsRes.isErr()) {
if (permissionsRes.error.type === "connector_rate_limit_error") {
return apiError(req, res, {
status_code: 429,
api_error: {
type: "rate_limit_error",
message:
"Rate limit error while retrieving the data source permissions",
},
});
}
return apiError(req, res, {
status_code: 500,
api_error: {
Expand Down
Loading