diff --git a/src/lib/account-region.ts b/src/lib/account-region.ts index 00bc9be5..5ea14bb7 100644 --- a/src/lib/account-region.ts +++ b/src/lib/account-region.ts @@ -119,6 +119,7 @@ async function probe( `${config["browserstack-username"]}:${config["browserstack-access-key"]}`, ).toString("base64"); let rejected = 0; + const answers: string[] = []; for (const [region, host] of PROBE_HOSTS) { try { const res = await apiClient.get({ @@ -129,10 +130,13 @@ async function probe( }); if (res.ok) return region; if (res.status === 401) rejected++; + answers.push(`${region} ${res.status}`); } catch (error) { logger.debug(`Region probe failed for ${host}: ${error}`); + answers.push(`${region} unreachable`); } } + logger.warn(`No region probe accepted the account: ${answers.join(", ")}`); return rejected === PROBE_HOSTS.length ? "rejected" : null; } diff --git a/src/lib/instrumentation.ts b/src/lib/instrumentation.ts index 699c6815..1ce360ad 100644 --- a/src/lib/instrumentation.ts +++ b/src/lib/instrumentation.ts @@ -102,11 +102,14 @@ function forDataRegion( account: AccountRegion | undefined, ): MCPEventPayload["event_properties"] { if (!isRestrictedRegion(account)) return props; + const keepsErrorText = account === "unknown"; return Object.fromEntries( Object.entries(props).filter( // A product name the agent typed is free text, so only a known product is kept. ([key, value]) => - USAGE_FIELDS.has(key) && (key !== "product" || isKnownProduct(value)), + (USAGE_FIELDS.has(key) && + (key !== "product" || isKnownProduct(value))) || + (keepsErrorText && key === "error_message"), ), ) as MCPEventPayload["event_properties"]; } diff --git a/tests/lib/account-region-behaviour.test.ts b/tests/lib/account-region-behaviour.test.ts index b101f27e..33b5c4a7 100644 --- a/tests/lib/account-region-behaviour.test.ts +++ b/tests/lib/account-region-behaviour.test.ts @@ -93,13 +93,13 @@ describe("the account's region", () => { }); it.each([ - ["eu", true], - ["unknown", true], - ["us", false], - ["in", true], + ["eu", false, false], + ["unknown", true, false], + ["us", true, true], + ["in", false, false], ])( - "limits telemetry outside a tool call for a %s account: %s", - async (region, limited) => { + "limits telemetry outside a tool call for a %s account", + async (region, errorText, freeText) => { regions.set("user", region); trackMCP("started", clientInfo, new Error("boom"), configFor("user"), { search_query: "checkout runs", @@ -107,8 +107,8 @@ describe("the account's region", () => { await settle(); const row = rows().at(-1); expect(row.tool_name).toBe("started"); - expect("error_message" in row).toBe(!limited); - expect("search_query" in row).toBe(!limited); + expect("error_message" in row).toBe(errorText); + expect("search_query" in row).toBe(freeText); }, ); diff --git a/tests/lib/instrumentation-grr-region.test.ts b/tests/lib/instrumentation-grr-region.test.ts index cbcf6aab..7806b67e 100644 --- a/tests/lib/instrumentation-grr-region.test.ts +++ b/tests/lib/instrumentation-grr-region.test.ts @@ -24,7 +24,44 @@ const lastRow = () => describe("telemetry by the account's region", () => { beforeEach(() => vi.clearAllMocks()); - it.each(["eu", "in", "unknown"])( + it("keeps only usage fields and the error text when the region is unknown", () => { + withAccountRegion("unknown", () => + trackMCP("invokeCapability", clientInfo, new Error("boom"), config, { + capability: "listTestRuns", + change_summary: "close the checkout regression run", + search_query: "checkout runs", + feedback_message: "it failed", + }), + ); + const row = lastRow(); + expect(row).toMatchObject({ + error_type: "Error", + error_message: "boom", + capability: "listTestRuns", + }); + for (const field of ["change_summary", "search_query", "feedback_message"]) + expect(row).not.toHaveProperty(field); + }); + + it("keeps the error text of a returned failure when the region is unknown", async () => { + await withAccountRegion("unknown", () => + withToolCall( + "listTestCases", + () => clientInfo, + config, + async () => ({ + content: [{ type: "text", text: "Project PR-1 not found" }], + isError: true, + }), + ), + ); + expect(lastRow()).toMatchObject({ + outcome: "error_result", + error_message: "Project PR-1 not found", + }); + }); + + it.each(["eu", "in"])( "drops free text but keeps usage fields for a %s account", (region) => { withAccountRegion(region as any, () => @@ -57,7 +94,7 @@ describe("telemetry by the account's region", () => { }, ); - it.each(["eu", "in", "unknown"])( + it.each(["eu", "in"])( "drops the error text of a returned failure for a %s account", async (region) => { await withAccountRegion(region as any, () =>