From 11959bb7b0d05d78dd97b61d80e6abed8a9ae612 Mon Sep 17 00:00:00 2001 From: Laurent Nguyen Date: Wed, 16 Sep 2026 14:30:19 +0200 Subject: [PATCH 1/2] Enhance logging and error handling in sdk read collections api (#2602) * Enhance logging and error handling in readCollections function * Add defensive measure to avoid breaking runtime. --------- Co-authored-by: Laurent Nguyen --- src/Common/dataAccess/readCollections.test.ts | 97 +++++++++++++++---- src/Common/dataAccess/readCollections.ts | 13 ++- 2 files changed, 91 insertions(+), 19 deletions(-) diff --git a/src/Common/dataAccess/readCollections.test.ts b/src/Common/dataAccess/readCollections.test.ts index 50348b574..d3998ecfc 100644 --- a/src/Common/dataAccess/readCollections.test.ts +++ b/src/Common/dataAccess/readCollections.test.ts @@ -1,13 +1,39 @@ jest.mock("../../Utils/arm/request"); jest.mock("../CosmosClient"); +jest.mock("../Logger"); +jest.mock("../ErrorHandlingUtils", () => ({ handleError: jest.fn() })); +jest.mock("../../Utils/NotificationConsoleUtils"); +import * as Logger from "Common/Logger"; import { AuthType } from "../../AuthType"; import { DatabaseAccount } from "../../Contracts/DataModels"; import { updateUserContext } from "../../UserContext"; +import { logConsoleProgress } from "../../Utils/NotificationConsoleUtils"; import { armRequest } from "../../Utils/arm/request"; import { client } from "../CosmosClient"; +import { handleError } from "../ErrorHandlingUtils"; import { readCollections } from "./readCollections"; describe("readCollections", () => { + const clearMessage = jest.fn(); + const fetchAll = jest.fn(); + const readAll = jest.fn(() => ({ fetchAll })); + const database = jest.fn(() => ({ containers: { readAll } })); + const diagnostics = { + clientSideRequestStatistics: { + requestDurationInMs: 123, + locationEndpointsContacted: ["https://test.documents.azure.com"], + retryDiagnostics: { failedAttempts: [{ statusCode: 429 }] }, + }, + diagnosticNode: { data: { responsePayload: "not-for-logging" } }, + }; + + beforeEach(() => { + jest.clearAllMocks(); + (logConsoleProgress as jest.Mock).mockReturnValue(clearMessage); + (client as jest.Mock).mockReturnValue({ database }); + updateUserContext({ authType: AuthType.MasterKey }); + }); + beforeAll(() => { updateUserContext({ databaseAccount: { @@ -23,26 +49,61 @@ describe("readCollections", () => { }); await readCollections("database"); expect(armRequest).toHaveBeenCalled(); + expect(client).not.toHaveBeenCalled(); + expect(clearMessage).toHaveBeenCalledTimes(1); }); - it("should call SDK if not logged in with non-AAD method", async () => { - updateUserContext({ - authType: AuthType.MasterKey, + it("should log SDK request statistics and return collections for non-AAD authentication", async () => { + const resources = [{ id: "container" }]; + fetchAll.mockResolvedValue({ resources, diagnostics }); + + await expect(readCollections("database")).resolves.toBe(resources); + + expect(database).toHaveBeenCalledWith("database"); + expect(readAll).toHaveBeenCalledWith(); + expect(fetchAll).toHaveBeenCalledTimes(1); + expect(Logger.logInfo).toHaveBeenLastCalledWith( + expect.stringContaining(`diagnostics=${JSON.stringify(diagnostics.clientSideRequestStatistics)}`), + "readCollections", + ); + expect(JSON.stringify(jest.mocked(Logger.logInfo).mock.calls)).not.toContain("not-for-logging"); + expect(clearMessage).toHaveBeenCalledTimes(1); + }); + + it("should log SDK failure statistics and rethrow the original error", async () => { + const error = Object.assign(new Error("fetchAll failed"), { diagnostics }); + fetchAll.mockRejectedValue(error); + + await expect(readCollections("database")).rejects.toBe(error); + + expect(Logger.logError).toHaveBeenCalledWith( + `readCollections: fetchAll failed for database database, diagnostics=${JSON.stringify( + diagnostics.clientSideRequestStatistics, + )}`, + "readCollections", + ); + expect(handleError).toHaveBeenCalledWith( + error, + "ReadCollections", + "Error while querying containers for database database", + ); + expect(clearMessage).toHaveBeenCalledTimes(1); + }); + + it("should preserve error handling when diagnostics are unavailable", async () => { + const error = new Error("client initialization failed"); + (client as jest.Mock).mockImplementationOnce(() => { + throw error; }); - (client as jest.Mock).mockReturnValue({ - database: () => { - return { - containers: { - readAll: () => { - return { - fetchAll: (): unknown => [], - }; - }, - }, - }; - }, - }); - await readCollections("database"); - expect(client).toHaveBeenCalled(); + + await expect(readCollections("database")).rejects.toBe(error); + + expect(Logger.logError).not.toHaveBeenCalled(); + expect(handleError).toHaveBeenCalledWith( + error, + "ReadCollections", + "Error while querying containers for database database", + ); + expect(clearMessage).toHaveBeenCalledTimes(1); }); }); diff --git a/src/Common/dataAccess/readCollections.ts b/src/Common/dataAccess/readCollections.ts index d90832219..caf317a16 100644 --- a/src/Common/dataAccess/readCollections.ts +++ b/src/Common/dataAccess/readCollections.ts @@ -78,7 +78,9 @@ export async function readCollections(databaseId: string): Promise Date: Wed, 16 Sep 2026 10:24:59 -0700 Subject: [PATCH 2/2] Fix MongoDB 3.6 Cloud Shell compatibility (#2603) Use the MongoDB 3.6-compatible mongosh package for Mongo server version 3.6 while keeping the newer package for other accounts. This reads serverVersion from account metadata and adds a unit test covering the 3.6 path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Contracts/DataModels.ts | 5 +++++ .../ShellTypes/AbstractShellHandler.tsx | 15 +++++++++------ .../ShellTypes/MongoShellHandler.test.tsx | 14 ++++++++++++++ .../ShellTypes/MongoShellHandler.tsx | 2 +- 4 files changed, 29 insertions(+), 7 deletions(-) diff --git a/src/Contracts/DataModels.ts b/src/Contracts/DataModels.ts index 770ee7948..ad8cbdf38 100644 --- a/src/Contracts/DataModels.ts +++ b/src/Contracts/DataModels.ts @@ -39,6 +39,7 @@ export interface DatabaseAccountBackupPolicy { } export interface DatabaseAccountExtendedProperties { + apiProperties?: DatabaseAccountApiProperties; documentEndpoint?: string; disableLocalAuth?: boolean; tableEndpoint?: string; @@ -68,6 +69,10 @@ export interface DatabaseAccountExtendedProperties { enableAllVersionsAndDeletesChangeFeed?: boolean; } +export interface DatabaseAccountApiProperties { + serverVersion?: string; +} + export interface DatabaseAccountResponseLocation { documentEndpoint: string; failoverPriority: number; diff --git a/src/Explorer/Tabs/CloudShellTab/ShellTypes/AbstractShellHandler.tsx b/src/Explorer/Tabs/CloudShellTab/ShellTypes/AbstractShellHandler.tsx index 783dcd99b..3834538c3 100644 --- a/src/Explorer/Tabs/CloudShellTab/ShellTypes/AbstractShellHandler.tsx +++ b/src/Explorer/Tabs/CloudShellTab/ShellTypes/AbstractShellHandler.tsx @@ -26,6 +26,9 @@ export const EXIT_COMMAND_MONGO = ` printf "\\033[1;31mSession ended. Please clo */ export const DISABLE_TELEMETRY_COMMAND = `mongosh --nodb --quiet --eval 'disableTelemetry()'`; +const MONGOSH_PACKAGE_VERSION = "2.5.6"; +const MONGOSH_36_PACKAGE_VERSION = "1.10.6"; + /** * Abstract class that defines the interface for shell-specific handlers * in the CloudShell terminal implementation. Each supported shell type @@ -96,14 +99,14 @@ export abstract class AbstractShellHandler { * Each command runs conditionally only if mongosh * is not already present in the environment. */ - protected mongoShellSetupCommands(): string[] { - const PACKAGE_VERSION: string = "2.5.6"; + protected mongoShellSetupCommands(serverVersion?: string): string[] { + const packageVersion = serverVersion === "3.6" ? MONGOSH_36_PACKAGE_VERSION : MONGOSH_PACKAGE_VERSION; return [ "if ! command -v mongosh &> /dev/null; then echo '⚠️ mongosh not found. Installing...'; fi", - `if ! command -v mongosh &> /dev/null; then curl -LO https://downloads.mongodb.com/compass/mongosh-${PACKAGE_VERSION}-linux-x64.tgz; fi`, - `if ! command -v mongosh &> /dev/null; then tar -xvzf mongosh-${PACKAGE_VERSION}-linux-x64.tgz; fi`, - `if ! command -v mongosh &> /dev/null; then mkdir -p ~/mongosh/bin && mv mongosh-${PACKAGE_VERSION}-linux-x64/bin/mongosh ~/mongosh/bin/ && chmod +x ~/mongosh/bin/mongosh; fi`, - `if ! command -v mongosh &> /dev/null; then rm -rf mongosh-${PACKAGE_VERSION}-linux-x64 mongosh-${PACKAGE_VERSION}-linux-x64.tgz; fi`, + `if ! command -v mongosh &> /dev/null; then curl -LO https://downloads.mongodb.com/compass/mongosh-${packageVersion}-linux-x64.tgz; fi`, + `if ! command -v mongosh &> /dev/null; then tar -xvzf mongosh-${packageVersion}-linux-x64.tgz; fi`, + `if ! command -v mongosh &> /dev/null; then mkdir -p ~/mongosh/bin && mv mongosh-${packageVersion}-linux-x64/bin/mongosh ~/mongosh/bin/ && chmod +x ~/mongosh/bin/mongosh; fi`, + `if ! command -v mongosh &> /dev/null; then rm -rf mongosh-${packageVersion}-linux-x64 mongosh-${packageVersion}-linux-x64.tgz; fi`, "if ! command -v mongosh &> /dev/null; then echo 'export PATH=$HOME/mongosh/bin:$PATH' >> ~/.bashrc; fi", "if ! command -v mongosh &> /dev/null; then source ~/.bashrc; fi", ]; diff --git a/src/Explorer/Tabs/CloudShellTab/ShellTypes/MongoShellHandler.test.tsx b/src/Explorer/Tabs/CloudShellTab/ShellTypes/MongoShellHandler.test.tsx index 28a79d404..dcba243bf 100644 --- a/src/Explorer/Tabs/CloudShellTab/ShellTypes/MongoShellHandler.test.tsx +++ b/src/Explorer/Tabs/CloudShellTab/ShellTypes/MongoShellHandler.test.tsx @@ -5,6 +5,9 @@ import { MongoShellHandler } from "./MongoShellHandler"; // Define interfaces for type safety interface DatabaseAccountProperties { mongoEndpoint?: string; + apiProperties?: { + serverVersion?: string; + }; } interface DatabaseAccount { @@ -80,6 +83,17 @@ describe("MongoShellHandler", () => { expect(commands.length).toBe(7); expect(commands[1]).toContain("mongosh-2.5.6-linux-x64.tgz"); }); + + it("should download a MongoDB 3.6-compatible package for 3.6 accounts", () => { + const properties = (userContext as UserContextType).databaseAccount.properties; + const originalApiProperties = properties.apiProperties; + properties.apiProperties = { serverVersion: "3.6" }; + + const commands = mongoShellHandler.getSetUpCommands(); + + expect(commands[1]).toContain("mongosh-1.10.6-linux-x64.tgz"); + properties.apiProperties = originalApiProperties; + }); }); describe("getConnectionCommand", () => { diff --git a/src/Explorer/Tabs/CloudShellTab/ShellTypes/MongoShellHandler.tsx b/src/Explorer/Tabs/CloudShellTab/ShellTypes/MongoShellHandler.tsx index b41dbd6db..12869cb93 100644 --- a/src/Explorer/Tabs/CloudShellTab/ShellTypes/MongoShellHandler.tsx +++ b/src/Explorer/Tabs/CloudShellTab/ShellTypes/MongoShellHandler.tsx @@ -29,7 +29,7 @@ export class MongoShellHandler extends AbstractShellHandler { } public getSetUpCommands(): string[] { - return this.mongoShellSetupCommands(); + return this.mongoShellSetupCommands(userContext.databaseAccount?.properties.apiProperties?.serverVersion); } public getConnectionCommand(): string {