Skip to content

fix: improve uncaught exception for getAccessToken #224

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix: improve uncaught exception for getAccessToken #224

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
87fe0f3
improve uncaught exception for getToken
blva May 8, 2025
2dbc94a
update
blva May 8, 2025
bf8fae5
one more test case
blva May 8, 2025
b99ba2c
address comment
blva May 8, 2025
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
84 changes: 46 additions & 38 deletions src/common/atlas/apiClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,8 @@ export class ApiClient {
return !!(this.oauth2Client && this.accessToken);
}

public async hasValidAccessToken(): Promise<boolean> {
const accessToken = await this.getAccessToken();
return accessToken !== undefined;
public async validateAccessToken(): Promise<void> {
await this.getAccessToken();
Copy link
Collaborator

Choose a reason for hiding this comment

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

not sure if I got it right but this can end up just not finding the access token right? so it'd effectively not validate it always?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it validates if it exists otherwise it will just return undefined, which is fine

}

public async getIpInfo(): Promise<{
Expand Down Expand Up @@ -119,48 +118,57 @@ export class ApiClient {
}>;
}

async sendEvents(events: TelemetryEvent<CommonProperties>[]): Promise<void> {
const headers: Record<string, string> = {
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}`;
public async sendEvents(events: TelemetryEvent<CommonProperties>[]): Promise<void> {
if (!this.options.credentials) {
await this.sendUnauthEvents(events);
return;
}

try {
const response = await fetch(authUrl, {
method: "POST",
headers,
body: JSON.stringify(events),
});

if (response.ok) {
return;
try {
await this.sendAuthEvents(events);
} catch (error) {
if (error instanceof ApiClientError) {
if (error.response.status !== 401) {
throw error;
}
}

// If anything other than 401, throw the error
if (response.status !== 401) {
throw await ApiClientError.fromResponse(response);
}
// 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);
}
}

// 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) {
throw error;
}
private async sendAuthEvents(events: TelemetryEvent<CommonProperties>[]): Promise<void> {
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",
headers: {
Accept: "application/json",
"Content-Type": "application/json",
"User-Agent": this.options.userAgent,
Authorization: `Bearer ${accessToken}`,
},
body: JSON.stringify(events),
});

// For 401 errors, fall through to unauthenticated endpoint
delete headers["Authorization"];
}
if (!response.ok) {
throw await ApiClientError.fromResponse(response);
}
}

private async sendUnauthEvents(events: TelemetryEvent<CommonProperties>[]): Promise<void> {
const headers: Record<string, string> = {
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",
Expand Down
2 changes: 1 addition & 1 deletion src/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion tests/integration/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ export function setupIntegrationTest(getUserConfig: () => UserConfig): Integrati
// Mock hasValidAccessToken for tests
if (userConfig.apiClientId && userConfig.apiClientSecret) {
const mockFn = jest.fn<() => Promise<boolean>>().mockResolvedValue(true);
session.apiClient.hasValidAccessToken = mockFn;
// @ts-expect-error accessing private property for testing
session.apiClient.validateAccessToken = mockFn;
}

userConfig.telemetry = "disabled";
Expand Down
27 changes: 24 additions & 3 deletions tests/unit/apiClient.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 }));

Expand All @@ -114,12 +114,33 @@ 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);

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 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);

Expand Down
Loading