Skip to content

Commit c339675

Browse files
authored
fix: improve uncaught exception for getAccessToken (#224)
1 parent 28b4dbe commit c339675

File tree

4 files changed

+73
-43
lines changed

4 files changed

+73
-43
lines changed

src/common/atlas/apiClient.ts

Lines changed: 46 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,8 @@ export class ApiClient {
8989
return !!(this.oauth2Client && this.accessToken);
9090
}
9191

92-
public async hasValidAccessToken(): Promise<boolean> {
93-
const accessToken = await this.getAccessToken();
94-
return accessToken !== undefined;
92+
public async validateAccessToken(): Promise<void> {
93+
await this.getAccessToken();
9594
}
9695

9796
public async getIpInfo(): Promise<{
@@ -119,48 +118,57 @@ export class ApiClient {
119118
}>;
120119
}
121120

122-
async sendEvents(events: TelemetryEvent<CommonProperties>[]): Promise<void> {
123-
const headers: Record<string, string> = {
124-
Accept: "application/json",
125-
"Content-Type": "application/json",
126-
"User-Agent": this.options.userAgent,
127-
};
128-
129-
const accessToken = await this.getAccessToken();
130-
if (accessToken) {
131-
const authUrl = new URL("api/private/v1.0/telemetry/events", this.options.baseUrl);
132-
headers["Authorization"] = `Bearer ${accessToken}`;
121+
public async sendEvents(events: TelemetryEvent<CommonProperties>[]): Promise<void> {
122+
if (!this.options.credentials) {
123+
await this.sendUnauthEvents(events);
124+
return;
125+
}
133126

134-
try {
135-
const response = await fetch(authUrl, {
136-
method: "POST",
137-
headers,
138-
body: JSON.stringify(events),
139-
});
140-
141-
if (response.ok) {
142-
return;
127+
try {
128+
await this.sendAuthEvents(events);
129+
} catch (error) {
130+
if (error instanceof ApiClientError) {
131+
if (error.response.status !== 401) {
132+
throw error;
143133
}
134+
}
144135

145-
// If anything other than 401, throw the error
146-
if (response.status !== 401) {
147-
throw await ApiClientError.fromResponse(response);
148-
}
136+
// send unauth events if any of the following are true:
137+
// 1: the token is not valid (not ApiClientError)
138+
// 2: if the api responded with 401 (ApiClientError with status 401)
139+
await this.sendUnauthEvents(events);
140+
}
141+
}
149142

150-
// For 401, fall through to unauthenticated endpoint
151-
delete headers["Authorization"];
152-
} catch (error) {
153-
// If the error is not a 401, rethrow it
154-
if (!(error instanceof ApiClientError) || error.response.status !== 401) {
155-
throw error;
156-
}
143+
private async sendAuthEvents(events: TelemetryEvent<CommonProperties>[]): Promise<void> {
144+
const accessToken = await this.getAccessToken();
145+
if (!accessToken) {
146+
throw new Error("No access token available");
147+
}
148+
const authUrl = new URL("api/private/v1.0/telemetry/events", this.options.baseUrl);
149+
const response = await fetch(authUrl, {
150+
method: "POST",
151+
headers: {
152+
Accept: "application/json",
153+
"Content-Type": "application/json",
154+
"User-Agent": this.options.userAgent,
155+
Authorization: `Bearer ${accessToken}`,
156+
},
157+
body: JSON.stringify(events),
158+
});
157159

158-
// For 401 errors, fall through to unauthenticated endpoint
159-
delete headers["Authorization"];
160-
}
160+
if (!response.ok) {
161+
throw await ApiClientError.fromResponse(response);
161162
}
163+
}
164+
165+
private async sendUnauthEvents(events: TelemetryEvent<CommonProperties>[]): Promise<void> {
166+
const headers: Record<string, string> = {
167+
Accept: "application/json",
168+
"Content-Type": "application/json",
169+
"User-Agent": this.options.userAgent,
170+
};
162171

163-
// Send to unauthenticated endpoint (either as fallback from 401 or direct if no token)
164172
const unauthUrl = new URL("api/private/unauth/telemetry/events", this.options.baseUrl);
165173
const response = await fetch(unauthUrl, {
166174
method: "POST",

src/server.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ export class Server {
188188

189189
if (this.userConfig.apiClientId && this.userConfig.apiClientSecret) {
190190
try {
191-
await this.session.apiClient.hasValidAccessToken();
191+
await this.session.apiClient.validateAccessToken();
192192
} catch (error) {
193193
if (this.userConfig.connectionString === undefined) {
194194
console.error("Failed to validate MongoDB Atlas the credentials from the config: ", error);

tests/integration/helpers.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ export function setupIntegrationTest(getUserConfig: () => UserConfig): Integrati
6161
// Mock hasValidAccessToken for tests
6262
if (userConfig.apiClientId && userConfig.apiClientSecret) {
6363
const mockFn = jest.fn<() => Promise<boolean>>().mockResolvedValue(true);
64-
session.apiClient.hasValidAccessToken = mockFn;
64+
// @ts-expect-error accessing private property for testing
65+
session.apiClient.validateAccessToken = mockFn;
6566
}
6667

6768
userConfig.telemetry = "disabled";

tests/unit/apiClient.test.ts

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ describe("ApiClient", () => {
9595
});
9696

9797
describe("sendEvents", () => {
98-
it("should send events to authenticated endpoint when token is available", async () => {
98+
it("should send events to authenticated endpoint when token is available and valid", async () => {
9999
const mockFetch = jest.spyOn(global, "fetch");
100100
mockFetch.mockResolvedValueOnce(new Response(null, { status: 200 }));
101101

@@ -114,12 +114,33 @@ describe("ApiClient", () => {
114114
});
115115
});
116116

117-
it("should fall back to unauthenticated endpoint when token is not available", async () => {
117+
it("should fall back to unauthenticated endpoint when token is not available via exception", async () => {
118118
const mockFetch = jest.spyOn(global, "fetch");
119119
mockFetch.mockResolvedValueOnce(new Response(null, { status: 200 }));
120120

121121
// @ts-expect-error accessing private property for testing
122-
apiClient.getAccessToken = jest.fn().mockResolvedValue(undefined);
122+
apiClient.getAccessToken = jest.fn().mockRejectedValue(new Error("No access token available"));
123+
124+
await apiClient.sendEvents(mockEvents);
125+
126+
const url = new URL("api/private/unauth/telemetry/events", "https://api.test.com");
127+
expect(mockFetch).toHaveBeenCalledWith(url, {
128+
method: "POST",
129+
headers: {
130+
"Content-Type": "application/json",
131+
Accept: "application/json",
132+
"User-Agent": "test-user-agent",
133+
},
134+
body: JSON.stringify(mockEvents),
135+
});
136+
});
137+
138+
it("should fall back to unauthenticated endpoint when token is undefined", async () => {
139+
const mockFetch = jest.spyOn(global, "fetch");
140+
mockFetch.mockResolvedValueOnce(new Response(null, { status: 200 }));
141+
142+
// @ts-expect-error accessing private property for testing
143+
apiClient.getAccessToken = jest.fn().mockReturnValueOnce(undefined);
123144

124145
await apiClient.sendEvents(mockEvents);
125146

0 commit comments

Comments
 (0)