From 87fe0f359d3c8bda421f613ef9b6b639035a6bd6 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Thu, 8 May 2025 17:52:23 +0100 Subject: [PATCH 1/4] improve uncaught exception for getToken --- src/common/atlas/apiClient.ts | 81 ++++++++++++++++++----------------- 1 file changed, 42 insertions(+), 39 deletions(-) diff --git a/src/common/atlas/apiClient.ts b/src/common/atlas/apiClient.ts index 78bd688d..8e9138fc 100644 --- a/src/common/atlas/apiClient.ts +++ b/src/common/atlas/apiClient.ts @@ -89,9 +89,8 @@ export class ApiClient { return !!(this.oauth2Client && this.accessToken); } - public async hasValidAccessToken(): Promise { - const accessToken = await this.getAccessToken(); - return accessToken !== undefined; + public async validateAccessToken(): Promise { + await this.getAccessToken(); } public async getIpInfo(): Promise<{ @@ -119,48 +118,52 @@ export class ApiClient { }>; } - async sendEvents(events: TelemetryEvent[]): Promise { - const headers: Record = { - Accept: "application/json", - "Content-Type": "application/json", - "User-Agent": this.options.userAgent, - }; - - const accessToken = await this.getAccessToken(); - if (accessToken) { - const authUrl = new URL("api/private/v1.0/telemetry/events", this.options.baseUrl); - headers["Authorization"] = `Bearer ${accessToken}`; - - try { - const response = await fetch(authUrl, { - method: "POST", - headers, - body: JSON.stringify(events), - }); - - if (response.ok) { - return; - } - - // If anything other than 401, throw the error - if (response.status !== 401) { - throw await ApiClientError.fromResponse(response); - } + public async sendEvents(events: TelemetryEvent[]): Promise { + if (!this.hasCredentials()) { + await this.sendUnauthEvents(events); + return; + } - // For 401, fall through to unauthenticated endpoint - delete headers["Authorization"]; - } catch (error) { - // If the error is not a 401, rethrow it - if (!(error instanceof ApiClientError) || error.response.status !== 401) { + try { + await this.validateAccessToken(); + await this.sendAuthEvents(events); + } catch (error) { + if (error instanceof ApiClientError) { + if (error.response.status !== 401) { throw error; } - - // For 401 errors, fall through to unauthenticated endpoint - delete headers["Authorization"]; } + // send unauth events if the token is not valid + await this.sendUnauthEvents(events); + return; } + } + + private async sendAuthEvents(events: TelemetryEvent[]): Promise { + const authUrl = new URL("api/private/v1.0/telemetry/events", this.options.baseUrl); + const response = await fetch(authUrl, { + method: "POST", + headers: { + Accept: "application/json", + "Content-Type": "application/json", + "User-Agent": this.options.userAgent, + Authorization: `Bearer ${this.accessToken}`, + }, + body: JSON.stringify(events), + }); + + if (!response.ok) { + throw await ApiClientError.fromResponse(response); + } + } + + private async sendUnauthEvents(events: TelemetryEvent[]): Promise { + const headers: Record = { + Accept: "application/json", + "Content-Type": "application/json", + "User-Agent": this.options.userAgent, + }; - // Send to unauthenticated endpoint (either as fallback from 401 or direct if no token) const unauthUrl = new URL("api/private/unauth/telemetry/events", this.options.baseUrl); const response = await fetch(unauthUrl, { method: "POST", From 2dbc94af13e8f6d73a261bc6f85f75862c814e6f Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Thu, 8 May 2025 18:14:48 +0100 Subject: [PATCH 2/4] update --- src/common/atlas/apiClient.ts | 15 ++++++++++----- src/server.ts | 2 +- tests/integration/helpers.ts | 3 ++- tests/unit/apiClient.test.ts | 6 +++--- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/common/atlas/apiClient.ts b/src/common/atlas/apiClient.ts index 8e9138fc..4cbd34d6 100644 --- a/src/common/atlas/apiClient.ts +++ b/src/common/atlas/apiClient.ts @@ -119,13 +119,12 @@ export class ApiClient { } public async sendEvents(events: TelemetryEvent[]): Promise { - if (!this.hasCredentials()) { + if (!this.options.credentials) { await this.sendUnauthEvents(events); return; } try { - await this.validateAccessToken(); await this.sendAuthEvents(events); } catch (error) { if (error instanceof ApiClientError) { @@ -133,13 +132,19 @@ export class ApiClient { throw error; } } - // send unauth events if the token is not valid + + // send unauth events if any of the following are true: + // 1: the token is not valid (not ApiClientError) + // 2: if the api responded with 401 (ApiClientError with status 401) await this.sendUnauthEvents(events); - return; } } private async sendAuthEvents(events: TelemetryEvent[]): Promise { + const accessToken = await this.getAccessToken(); + if (!accessToken) { + throw new Error("No access token available"); + } const authUrl = new URL("api/private/v1.0/telemetry/events", this.options.baseUrl); const response = await fetch(authUrl, { method: "POST", @@ -147,7 +152,7 @@ export class ApiClient { Accept: "application/json", "Content-Type": "application/json", "User-Agent": this.options.userAgent, - Authorization: `Bearer ${this.accessToken}`, + Authorization: `Bearer ${accessToken}`, }, body: JSON.stringify(events), }); diff --git a/src/server.ts b/src/server.ts index 091ebd79..b0e8e19c 100644 --- a/src/server.ts +++ b/src/server.ts @@ -188,7 +188,7 @@ export class Server { if (this.userConfig.apiClientId && this.userConfig.apiClientSecret) { try { - await this.session.apiClient.hasValidAccessToken(); + await this.session.apiClient.validateAccessToken(); } catch (error) { if (this.userConfig.connectionString === undefined) { console.error("Failed to validate MongoDB Atlas the credentials from the config: ", error); diff --git a/tests/integration/helpers.ts b/tests/integration/helpers.ts index fb79d08d..9d529376 100644 --- a/tests/integration/helpers.ts +++ b/tests/integration/helpers.ts @@ -61,7 +61,8 @@ export function setupIntegrationTest(getUserConfig: () => UserConfig): Integrati // Mock hasValidAccessToken for tests if (userConfig.apiClientId && userConfig.apiClientSecret) { const mockFn = jest.fn<() => Promise>().mockResolvedValue(true); - session.apiClient.hasValidAccessToken = mockFn; + // @ts-expect-error accessing private property for testing + session.apiClient.validateAccessToken = mockFn; } userConfig.telemetry = "disabled"; diff --git a/tests/unit/apiClient.test.ts b/tests/unit/apiClient.test.ts index a704e6b7..8135402c 100644 --- a/tests/unit/apiClient.test.ts +++ b/tests/unit/apiClient.test.ts @@ -95,7 +95,7 @@ describe("ApiClient", () => { }); describe("sendEvents", () => { - it("should send events to authenticated endpoint when token is available", async () => { + it("should send events to authenticated endpoint when token is available and valid", async () => { const mockFetch = jest.spyOn(global, "fetch"); mockFetch.mockResolvedValueOnce(new Response(null, { status: 200 })); @@ -114,12 +114,12 @@ describe("ApiClient", () => { }); }); - it("should fall back to unauthenticated endpoint when token is not available", async () => { + it("should fall back to unauthenticated endpoint when token is not available via exception", async () => { const mockFetch = jest.spyOn(global, "fetch"); mockFetch.mockResolvedValueOnce(new Response(null, { status: 200 })); // @ts-expect-error accessing private property for testing - apiClient.getAccessToken = jest.fn().mockResolvedValue(undefined); + apiClient.getAccessToken = jest.fn().mockRejectedValue(new Error("No access token available")); await apiClient.sendEvents(mockEvents); From bf8fae500f73fe93af49776a317be8fafd58123c Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Thu, 8 May 2025 18:22:51 +0100 Subject: [PATCH 3/4] one more test case --- tests/unit/apiClient.test.ts | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/unit/apiClient.test.ts b/tests/unit/apiClient.test.ts index 8135402c..1921a283 100644 --- a/tests/unit/apiClient.test.ts +++ b/tests/unit/apiClient.test.ts @@ -135,6 +135,27 @@ describe("ApiClient", () => { }); }); + it("should fall back to unauthenticated endpoint when token is undefined", async () => { + const mockFetch = jest.spyOn(global, "fetch"); + mockFetch.mockResolvedValueOnce(new Response(null, { status: 200 })); + + // @ts-expect-error accessing private property for testing + apiClient.getAccessToken = jest.fn().mockRejectedValue(undefined); + + await apiClient.sendEvents(mockEvents); + + const url = new URL("api/private/unauth/telemetry/events", "https://api.test.com"); + expect(mockFetch).toHaveBeenCalledWith(url, { + method: "POST", + headers: { + "Content-Type": "application/json", + Accept: "application/json", + "User-Agent": "test-user-agent", + }, + body: JSON.stringify(mockEvents), + }); + }); + it("should fall back to unauthenticated endpoint on 401 error", async () => { const mockFetch = jest.spyOn(global, "fetch"); mockFetch From b99ba2cb3892237c48025fac024e3c4d2343bf34 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Thu, 8 May 2025 18:30:42 +0100 Subject: [PATCH 4/4] address comment --- tests/unit/apiClient.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/apiClient.test.ts b/tests/unit/apiClient.test.ts index 1921a283..6b9fd427 100644 --- a/tests/unit/apiClient.test.ts +++ b/tests/unit/apiClient.test.ts @@ -140,7 +140,7 @@ describe("ApiClient", () => { mockFetch.mockResolvedValueOnce(new Response(null, { status: 200 })); // @ts-expect-error accessing private property for testing - apiClient.getAccessToken = jest.fn().mockRejectedValue(undefined); + apiClient.getAccessToken = jest.fn().mockReturnValueOnce(undefined); await apiClient.sendEvents(mockEvents);