Skip to content
Open
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
56 changes: 41 additions & 15 deletions packages/appkit/src/connectors/genie/client.ts
Copy link
Member

Choose a reason for hiding this comment

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

General question: maybe it's worth to reach to Genie Team and ask for API extension in case of error responses? WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reached out to them to share our problem and ask if something could be done. Although I haven't received a response yet, I doubt they will be able to implement anything on their side on the short-term, so maybe we can start with this, and move forward with a better solution

Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ const Time = SDK.Time ?? (SDK as any).default.Time;

const logger = createLogger("connectors:genie");

const GenieErrors = {
SPACE_ACCESS_DENIED: "You don't have access to this Genie Space.",
TABLE_PERMISSIONS:
"You may not have access to the data tables. Please verify your table permissions.",
REQUEST_FAILED: "Genie request failed",
QUERY_RESULT_FAILED: "Failed to fetch query result",
} as const;

type CreateMessageWaiter = Waiter<GenieMessage, GenieMessage>;

export interface GenieConnectorConfig {
Expand Down Expand Up @@ -54,6 +62,23 @@ function toMessageResponse(message: GenieMessage): GenieMessageResponse {
};
}

function classifyGenieError(error: unknown): string {
const message = error instanceof Error ? error.message : String(error);

if (message.includes("RESOURCE_DOES_NOT_EXIST")) {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can have an object which defines what's the API returns and what we display? WDYT? It will be easier to maintain it for future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, I defined these in an object

return GenieErrors.SPACE_ACCESS_DENIED;
}

if (
message.includes("failed to reach COMPLETED state") &&
message.includes("FAILED")
) {
return GenieErrors.TABLE_PERMISSIONS;
}

return message || GenieErrors.REQUEST_FAILED;
}

export class GenieConnector {
private readonly config: Required<GenieConnectorConfig>;

Expand Down Expand Up @@ -206,11 +231,13 @@ export class GenieConnector {
messageResponse,
);
} catch (error) {
logger.error("Genie message error: %O", error);
yield {
type: "error",
error: error instanceof Error ? error.message : "Genie request failed",
};
logger.error(
"Genie message error (spaceId=%s, conversationId=%s): %O",
spaceId,
conversationId ?? "new",
error,
);
yield { type: "error", error: classifyGenieError(error) };
}
}

Expand Down Expand Up @@ -248,7 +275,7 @@ export class GenieConnector {
);
yield {
type: "error",
error: `Failed to fetch query result for attachment ${att.attachmentId}`,
error: `${GenieErrors.QUERY_RESULT_FAILED} for attachment ${att.attachmentId}`,
};
}
}
Expand Down Expand Up @@ -324,20 +351,19 @@ export class GenieConnector {
error:
result.reason instanceof Error
? result.reason.message
: "Failed to fetch query result",
: GenieErrors.QUERY_RESULT_FAILED,
};
}
}
}
} catch (error) {
logger.error("Genie getConversation error: %O", error);
yield {
type: "error",
error:
error instanceof Error
? error.message
: "Failed to fetch conversation",
};
logger.error(
"Genie getConversation error (spaceId=%s, conversationId=%s): %O",
spaceId,
conversationId,
error,
);
yield { type: "error", error: classifyGenieError(error) };
}
}

Expand Down
107 changes: 107 additions & 0 deletions packages/appkit/src/plugins/genie/tests/genie.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -921,6 +921,113 @@ describe("Genie Plugin", () => {
});
});

describe("error classification", () => {
test("should return user-friendly message for RESOURCE_DOES_NOT_EXIST error", async () => {
mockGenieService.startConversation.mockRejectedValue(
new Error(
"RESOURCE_DOES_NOT_EXIST: No Genie space found with id test-space-id",
),
);

const plugin = new GeniePlugin(config);
const { router, getHandler } = createMockRouter();

plugin.injectRoutes(router);

const handler = getHandler("POST", "/:alias/messages");
const mockReq = createMockRequest({
params: { alias: "myspace" },
body: { content: "test question" },
headers: {
"x-forwarded-access-token": "user-token",
"x-forwarded-user": "user-1",
},
});
const mockRes = createMockResponse();

await handler(mockReq, mockRes);

const writeCalls = mockRes.write.mock.calls.map((call: any[]) => call[0]);
const allWritten = writeCalls.join("");

expect(allWritten).toContain(
"You don't have access to this Genie Space.",
);
expect(mockRes.end).toHaveBeenCalled();
});

test("should return user-friendly message for FAILED state error (table access denied)", async () => {
mockGenieService.startConversation.mockRejectedValue(
new Error(
"failed to reach COMPLETED state, got FAILED: [object Object]",
),
);

const plugin = new GeniePlugin(config);
const { router, getHandler } = createMockRouter();

plugin.injectRoutes(router);

const handler = getHandler("POST", "/:alias/messages");
const mockReq = createMockRequest({
params: { alias: "myspace" },
body: { content: "test question" },
headers: {
"x-forwarded-access-token": "user-token",
"x-forwarded-user": "user-1",
},
});
const mockRes = createMockResponse();

await handler(mockReq, mockRes);

const writeCalls = mockRes.write.mock.calls.map((call: any[]) => call[0]);
const allWritten = writeCalls.join("");

expect(allWritten).toContain(
"You may not have access to the data tables. Please verify your table permissions.",
);
expect(mockRes.end).toHaveBeenCalled();
});

test("should return user-friendly message for RESOURCE_DOES_NOT_EXIST on getConversation", async () => {
mockGenieService.listConversationMessages.mockRejectedValue(
new Error(
"RESOURCE_DOES_NOT_EXIST: No Genie space found with id test-space-id",
),
);

const plugin = new GeniePlugin(config);
const { router, getHandler } = createMockRouter();

plugin.injectRoutes(router);

const handler = getHandler(
"GET",
"/:alias/conversations/:conversationId",
);
const mockReq = createMockRequest({
params: { alias: "myspace", conversationId: "conv-123" },
query: {},
headers: {
"x-forwarded-access-token": "user-token",
"x-forwarded-user": "user-1",
},
});
const mockRes = createMockResponse();

await handler(mockReq, mockRes);

const writeCalls = mockRes.write.mock.calls.map((call: any[]) => call[0]);
const allWritten = writeCalls.join("");

expect(allWritten).toContain(
"You don't have access to this Genie Space.",
);
expect(mockRes.end).toHaveBeenCalled();
});
});

describe("default spaces from DATABRICKS_GENIE_SPACE_ID", () => {
test("should use env var as default space when spaces is omitted", async () => {
process.env.DATABRICKS_GENIE_SPACE_ID = "env-space-id";
Expand Down