mirror of
https://github.com/Azure/cosmos-explorer.git
synced 2026-09-19 09:02:41 +01:00
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 <languye@microsoft.com>
This commit is contained in:
@@ -1,13 +1,39 @@
|
|||||||
jest.mock("../../Utils/arm/request");
|
jest.mock("../../Utils/arm/request");
|
||||||
jest.mock("../CosmosClient");
|
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 { AuthType } from "../../AuthType";
|
||||||
import { DatabaseAccount } from "../../Contracts/DataModels";
|
import { DatabaseAccount } from "../../Contracts/DataModels";
|
||||||
import { updateUserContext } from "../../UserContext";
|
import { updateUserContext } from "../../UserContext";
|
||||||
|
import { logConsoleProgress } from "../../Utils/NotificationConsoleUtils";
|
||||||
import { armRequest } from "../../Utils/arm/request";
|
import { armRequest } from "../../Utils/arm/request";
|
||||||
import { client } from "../CosmosClient";
|
import { client } from "../CosmosClient";
|
||||||
|
import { handleError } from "../ErrorHandlingUtils";
|
||||||
import { readCollections } from "./readCollections";
|
import { readCollections } from "./readCollections";
|
||||||
|
|
||||||
describe("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(() => {
|
beforeAll(() => {
|
||||||
updateUserContext({
|
updateUserContext({
|
||||||
databaseAccount: {
|
databaseAccount: {
|
||||||
@@ -23,26 +49,61 @@ describe("readCollections", () => {
|
|||||||
});
|
});
|
||||||
await readCollections("database");
|
await readCollections("database");
|
||||||
expect(armRequest).toHaveBeenCalled();
|
expect(armRequest).toHaveBeenCalled();
|
||||||
|
expect(client).not.toHaveBeenCalled();
|
||||||
|
expect(clearMessage).toHaveBeenCalledTimes(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("should call SDK if not logged in with non-AAD method", async () => {
|
it("should log SDK request statistics and return collections for non-AAD authentication", async () => {
|
||||||
updateUserContext({
|
const resources = [{ id: "container" }];
|
||||||
authType: AuthType.MasterKey,
|
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: () => {
|
await expect(readCollections("database")).rejects.toBe(error);
|
||||||
return {
|
|
||||||
containers: {
|
expect(Logger.logError).not.toHaveBeenCalled();
|
||||||
readAll: () => {
|
expect(handleError).toHaveBeenCalledWith(
|
||||||
return {
|
error,
|
||||||
fetchAll: (): unknown => [],
|
"ReadCollections",
|
||||||
};
|
"Error while querying containers for database database",
|
||||||
},
|
);
|
||||||
},
|
expect(clearMessage).toHaveBeenCalledTimes(1);
|
||||||
};
|
|
||||||
},
|
|
||||||
});
|
|
||||||
await readCollections("database");
|
|
||||||
expect(client).toHaveBeenCalled();
|
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -78,7 +78,9 @@ export async function readCollections(databaseId: string): Promise<DataModels.Co
|
|||||||
const sdkResponse = await client().database(databaseId).containers.readAll().fetchAll();
|
const sdkResponse = await client().database(databaseId).containers.readAll().fetchAll();
|
||||||
Logger.logInfo(
|
Logger.logInfo(
|
||||||
`readCollections: fetchAll completed for database ${databaseId}, count=${sdkResponse.resources
|
`readCollections: fetchAll completed for database ${databaseId}, count=${sdkResponse.resources
|
||||||
?.length}, durationMs=${Date.now() - fetchAllStart}`,
|
?.length}, durationMs=${Date.now() - fetchAllStart}, diagnostics=${JSON.stringify(
|
||||||
|
sdkResponse.diagnostics?.clientSideRequestStatistics,
|
||||||
|
)}`,
|
||||||
"readCollections",
|
"readCollections",
|
||||||
);
|
);
|
||||||
traceSuccess(
|
traceSuccess(
|
||||||
@@ -88,6 +90,15 @@ export async function readCollections(databaseId: string): Promise<DataModels.Co
|
|||||||
);
|
);
|
||||||
return sdkResponse.resources as DataModels.Collection[];
|
return sdkResponse.resources as DataModels.Collection[];
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
|
const diagnostics = error instanceof Error && "diagnostics" in error ? error.diagnostics : undefined;
|
||||||
|
if (diagnostics && typeof diagnostics === "object" && "clientSideRequestStatistics" in diagnostics) {
|
||||||
|
Logger.logError(
|
||||||
|
`readCollections: fetchAll failed for database ${databaseId}, diagnostics=${JSON.stringify(
|
||||||
|
diagnostics.clientSideRequestStatistics,
|
||||||
|
)}`,
|
||||||
|
"readCollections",
|
||||||
|
);
|
||||||
|
}
|
||||||
traceFailure(Action.ReadCollections, { databaseId, error: error?.message }, startKey);
|
traceFailure(Action.ReadCollections, { databaseId, error: error?.message }, startKey);
|
||||||
handleError(error, "ReadCollections", `Error while querying containers for database ${databaseId}`);
|
handleError(error, "ReadCollections", `Error while querying containers for database ${databaseId}`);
|
||||||
throw error;
|
throw error;
|
||||||
|
|||||||
Reference in New Issue
Block a user