From 24f00ca02c8eeaaa5ef01ffdaacb1b15a6461039 Mon Sep 17 00:00:00 2001 From: Savio Dias Date: Tue, 6 Oct 2026 14:36:15 +0530 Subject: [PATCH] fix(automate): tolerate network logs with null entries or responses getFailureLogs threw "Cannot read properties of null" when a session's HAR contained a null entry, an entry without a response, or a body that was not a HAR file. Null entries are skipped, a missing response counts as a failed request, and a non-HAR body returns a message. Co-Authored-By: Claude Fable 5.1 --- src/tools/failurelogs-utils/automate.ts | 19 ++-- tests/tools/failurelogs-network.test.ts | 131 ++++++++++++++++++++++++ 2 files changed, 143 insertions(+), 7 deletions(-) create mode 100644 tests/tools/failurelogs-network.test.ts diff --git a/src/tools/failurelogs-utils/automate.ts b/src/tools/failurelogs-utils/automate.ts index 5424c950..02d95d8c 100644 --- a/src/tools/failurelogs-utils/automate.ts +++ b/src/tools/failurelogs-utils/automate.ts @@ -30,13 +30,18 @@ export async function retrieveNetworkFailures( const validationError = validateLogResponse(response, "network logs"); if (validationError) return validationError.message!; - const networklogs: HarFile = response.data; - const failureEntries: HarEntry[] = networklogs.log.entries.filter( - (entry: HarEntry) => - entry.response.status === 0 || - entry.response.status >= 400 || - entry.response._error !== undefined, - ); + const networklogs: HarFile | undefined = response.data; + const entries = networklogs?.log?.entries; + if (!Array.isArray(entries)) { + return "No network logs found for this session"; + } + const failureEntries: HarEntry[] = entries.filter((entry: HarEntry) => { + if (!entry) return false; + const res = entry.response; + return ( + !res || res.status === 0 || res.status >= 400 || res._error !== undefined + ); + }); return failureEntries.length > 0 ? `Network Failures (${failureEntries.length} found):\n${wrapUntrusted( diff --git a/tests/tools/failurelogs-network.test.ts b/tests/tools/failurelogs-network.test.ts new file mode 100644 index 00000000..8c2bf507 --- /dev/null +++ b/tests/tools/failurelogs-network.test.ts @@ -0,0 +1,131 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("../../src/lib/apiClient.js", () => ({ + apiClient: { get: vi.fn() }, +})); +vi.mock("../../src/lib/get-auth.js", () => ({ + getBrowserStackAuth: () => "user:key", +})); +vi.mock("../../src/config", () => ({ + __esModule: true, + default: { browserstackUsername: "user", browserstackAccessKey: "key" }, +})); +vi.mock("../../src/lib/instrumentation", () => ({ trackMCP: vi.fn() })); + +import { apiClient } from "../../src/lib/apiClient.js"; +import { retrieveNetworkFailures } from "../../src/tools/failurelogs-utils/automate.js"; +import { getFailureLogs } from "../../src/tools/get-failure-logs.js"; + +const config = {} as any; +const respond = (data: unknown) => + vi + .mocked(apiClient.get) + .mockResolvedValue({ ok: true, status: 200, data } as any); + +const entry = (response: unknown, url = "https://app.example/api") => ({ + startedDateTime: "2026-10-05T10:00:00Z", + time: 12, + request: { method: "GET", url, queryString: [] }, + response, + serverIPAddress: "10.0.0.1", +}); + +describe("retrieveNetworkFailures", () => { + beforeEach(() => vi.mocked(apiClient.get).mockReset()); + + it("reports failed requests from a well-formed HAR", async () => { + respond({ + log: { + entries: [ + entry({ status: 200 }), + entry({ status: 500, statusText: "Server Error" }), + ], + }, + }); + const out = await retrieveNetworkFailures("s1", config); + expect(out).toContain("Network Failures (1 found)"); + expect(out).toContain("500"); + }); + + it("treats an entry with a null response as a failed request instead of throwing", async () => { + respond({ + log: { + entries: [ + entry(null, "https://app.example/aborted"), + entry({ status: 204 }), + ], + }, + }); + const out = await retrieveNetworkFailures("s1", config); + expect(out).toContain("Network Failures (1 found)"); + expect(out).toContain("https://app.example/aborted"); + }); + + it("ignores a null entry in the HAR instead of throwing", async () => { + respond({ log: { entries: [null, entry({ status: 404 })] } }); + const out = await retrieveNetworkFailures("s1", config); + expect(out).toContain("Network Failures (1 found)"); + expect(out).toContain("404"); + }); + + it("treats an entry with no response field the same way", async () => { + respond({ log: { entries: [entry(undefined)] } }); + await expect(retrieveNetworkFailures("s1", config)).resolves.toContain( + "Network Failures (1 found)", + ); + }); + + it.each([ + ["an empty object", {}], + ["a log without entries", { log: {} }], + ["a string body", "no logs"], + ["null", null], + ])( + "returns a message when the body is %s rather than a HAR file", + async (_label, body) => { + respond(body); + await expect(retrieveNetworkFailures("s1", config)).resolves.toBe( + "No network logs found for this session", + ); + }, + ); + + it("still reports no failures for a clean HAR", async () => { + respond({ + log: { entries: [entry({ status: 200 }), entry({ status: 302 })] }, + }); + await expect(retrieveNetworkFailures("s1", config)).resolves.toBe( + "No network failures found", + ); + }); +}); + +describe("getFailureLogs tool with a HAR that has null entries and responses", () => { + beforeEach(() => vi.mocked(apiClient.get).mockReset()); + + it("returns a normal result instead of an error", async () => { + respond({ + log: { + entries: [ + null, + entry(null, "https://app.example/aborted"), + entry({ status: 200 }), + ], + }, + }); + const server = { server: { getClientVersion: () => "vitest" } } as any; + const result = await getFailureLogs( + { + sessionId: "s1", + logTypes: ["networkLogs"], + sessionType: "automate", + } as any, + server, + ); + expect(result.isError).toBeFalsy(); + const text = result.content.map((c: any) => c.text).join("\n"); + expect(text).toContain("Network Failures (1 found)"); + expect(text).toContain("https://app.example/aborted"); + expect(text).not.toContain("Cannot read properties"); + }); +});