From 55e876353800c83003f343f8fc79a8e60c34e77b Mon Sep 17 00:00:00 2001 From: Jason Stirnaman Date: Tue, 28 Jul 2026 12:23:49 -0500 Subject: [PATCH 1/4] Add tests for write-path error handling and packaging bugs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four version-independent bugs, none previously covered: - handleWriteError drops InfluxDB's response body on the write path for 400/401/403/413/422, has no 503 arm, and renders parsed JSON bodies as [object Object] (write-error-core.test.ts, write-error-cloud.test.ts). Mirrors the query-error-core.test.ts pattern; handleQueryError already does this correctly. - write routing per product type was untested (write-routing.test.ts): which endpoint/transport each product type uses, and that an unknown product type rejects rather than silently defaulting. - zod is imported at runtime throughout src/tools/ but declared only in devDependencies, so a clean `npm i --omit=dev` breaks (packaging.test.ts). - ping()'s x-influxdb-version/x-influxdb-build header passthrough was untested (base-connection-ping.test.ts). - protocol-write.test.ts covers write_line_protocol's advertised schema, error surfacing over the protocol boundary, and health_check's hasAnySuccess bug (a constructed client reports HEALTHY even when both /ping and /health fail). test:integration now matches any *integration* test file instead of only integration.test.ts — query-error-integration.test.ts was never actually wired into the live-instance run. Several `describe.skip("[P] ...")` blocks are acceptance criteria for fixes not yet implemented; each is paired with an active test asserting today's (wrong) behavior, so implementing the fix flips both at once. Split off a broader InfluxDB 3.11 compatibility pass: the version-gated parts (live-instance tests, the 3.11 tool-surface guardrail, open questions) moved to the influxdb-3.11-compat-patch branch instead. --- package.json | 2 +- tests/README.md | 50 ++++++ tests/base-connection-ping.test.ts | 85 ++++++++++ tests/fixtures/write-errors.ts | 242 ++++++++++++++++++++++++++++ tests/helpers/write-service.ts | 85 ++++++++++ tests/packaging.test.ts | 112 +++++++++++++ tests/protocol-write.test.ts | 227 ++++++++++++++++++++++++++ tests/write-error-cloud.test.ts | 160 +++++++++++++++++++ tests/write-error-core.test.ts | 246 +++++++++++++++++++++++++++++ tests/write-routing.test.ts | 169 ++++++++++++++++++++ 10 files changed, 1377 insertions(+), 1 deletion(-) create mode 100644 tests/README.md create mode 100644 tests/base-connection-ping.test.ts create mode 100644 tests/fixtures/write-errors.ts create mode 100644 tests/helpers/write-service.ts create mode 100644 tests/packaging.test.ts create mode 100644 tests/protocol-write.test.ts create mode 100644 tests/write-error-cloud.test.ts create mode 100644 tests/write-error-core.test.ts create mode 100644 tests/write-routing.test.ts diff --git a/package.json b/package.json index 5da64d7..eede7c7 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ "prepublishOnly": "npm run clean && npm run build", "pretest": "npm run build", "test": "vitest run", - "test:integration": "INFLUX_TEST_ENABLED=true vitest run tests/integration.test.ts", + "test:integration": "INFLUX_TEST_ENABLED=true vitest run integration", "test:watch": "vitest", "test:infra:up": "docker compose -f docker-compose.test.yml up -d --wait", "test:infra:down": "docker compose -f docker-compose.test.yml down", diff --git a/tests/README.md b/tests/README.md new file mode 100644 index 0000000..b936bdc --- /dev/null +++ b/tests/README.md @@ -0,0 +1,50 @@ +# Test suite + +Run `npm run build` first — tests spawn the compiled `build/index.js`. + +| Command | Runs | +| -------------------------- | ------------------------------------------------------------------ | +| `npm test` | Everything; live-instance tests self-skip | +| `npm run test:integration` | Every file matching `integration`, with `INFLUX_TEST_ENABLED=true` | + +## Conventions + +Three kinds of test live here, distinguished by how they are marked. + +**Active** — asserts what the code does today and must keep doing. Green. + +**`describe.skip("[P] …")`** — acceptance criteria for a planned change that +has not landed. Skipped so CI stays green, but each one fails against current +code, so un-skipping is all that is needed to drive the work. Where an +acceptance test has a paired active test asserting the opposite, the pair is +deliberate: the active test goes red when the fix lands, which forces the stale +characterization to be deleted rather than left behind. Two acceptance tests are +no-regression guards that pass both before and after, and say so in a comment. + +**`it.todo(…)`** — a test that cannot be written yet because the answer it would +assert against is unknown. Each names the question ID and what unblocks it. +These appear in every vitest run, so the open list stays visible in CI output. + +## Coverage for the write-path error and packaging fixes + +| Bug | Tests | +| ----------------------------------------------- | --------------------------------------------------- | +| Write path drops the InfluxDB error body | `write-error-core.test.ts`, `write-error-cloud.test.ts` | +| Which endpoint each product type writes to | `write-routing.test.ts` | +| `zod` is a runtime dependency, declared as dev | `packaging.test.ts` | +| `ping()` version/build header passthrough | `base-connection-ping.test.ts` | +| `write_line_protocol` contract, error surfacing | `protocol-write.test.ts` | + +None of these are version-gated — they're bugs in the current server, +independent of which InfluxDB release is on the other end. + +## Live-instance gates + +`INFLUX_TEST_ENABLED` covers any live instance; tests under it must pass on the +CI Core container. + +```bash +npm run test:infra:up +source env.test.example && npm run test:integration +npm run test:infra:down +``` diff --git a/tests/base-connection-ping.test.ts b/tests/base-connection-ping.test.ts new file mode 100644 index 0000000..3ef3ce1 --- /dev/null +++ b/tests/base-connection-ping.test.ts @@ -0,0 +1,85 @@ +/** + * `ping()` — version/build header parsing. + * + * The `x-influxdb-version` and `x-influxdb-build` response headers are fetched + * but never parsed, compared, or otherwise used. This pins the current + * passthrough behavior, independent of any specific InfluxDB version. + */ + +import { describe, it, expect, vi, afterEach } from "vitest"; +import { BaseConnectionService } from "../src/services/base-connection.service.js"; +import { InfluxProductType } from "../src/helpers/enums/influx-product-types.enum.js"; +import type { McpServerConfig } from "../src/config.js"; + +const TOKEN = "test-token-not-used"; + +function configFor(type: InfluxProductType): McpServerConfig { + return { + influx: { + url: "http://localhost:19999/", + token: TOKEN, + type, + cluster_id: + type === InfluxProductType.CloudDedicated + ? "00000000-0000-0000-0000-000000000000" + : undefined, + }, + server: { name: "influxdb-mcp-server", version: "test" }, + tools: { profile: "operator" }, + }; +} + +/** Stub `fetch` with a success response and return the recorded calls. */ +function stubFetchOk(headers: Record = {}) { + const fetchMock = vi.fn().mockResolvedValue({ + ok: true, + status: 200, + headers: { get: (name: string) => headers[name.toLowerCase()] ?? null }, + json: async () => ({ status: "pass" }), + }); + vi.stubGlobal("fetch", fetchMock); + return fetchMock; +} + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +describe("ping – version reporting, current behavior", () => { + it("returns the x-influxdb-version header unparsed", async () => { + const fetchMock = stubFetchOk({ + "x-influxdb-version": "3.11.0-0.rc.1", + "x-influxdb-build": "Enterprise", + }); + + const result = await new BaseConnectionService( + configFor(InfluxProductType.Enterprise), + ).ping(); + + expect(fetchMock).toHaveBeenCalled(); + expect(result).toEqual({ + ok: true, + version: "3.11.0-0.rc.1", + build: "Enterprise", + }); + }); + + it('reports build "Other" when only a version header is present', async () => { + stubFetchOk({ "x-influxdb-version": "3.11.0" }); + + const result = await new BaseConnectionService( + configFor(InfluxProductType.Core), + ).ping(); + + expect(result).toEqual({ ok: true, version: "3.11.0", build: "Other" }); + }); + + it("reports no version at all when the header is absent", async () => { + stubFetchOk(); + const result = await new BaseConnectionService( + configFor(InfluxProductType.Enterprise), + ).ping(); + + expect(result).toEqual({ ok: true, version: undefined, build: undefined }); + }); +}); diff --git a/tests/fixtures/write-errors.ts b/tests/fixtures/write-errors.ts new file mode 100644 index 0000000..2015886 --- /dev/null +++ b/tests/fixtures/write-errors.ts @@ -0,0 +1,242 @@ +/** + * Write-path error fixtures for the InfluxDB 3.11 compatibility patch. + * + * Companion to `error-responses.ts`, which covers the query path. Shapes follow + * the same two families: + * + * Axios errors (Core/Enterprise/Clustered HTTP paths) + * error.response.status, error.response.statusText, error.response.data + * SDK HttpError (Cloud Dedicated/Serverless client paths) + * error.statusCode, error.statusMessage, error.body, error.json, error.message + * + * ── Provenance ────────────────────────────────────────────────────────────── + * + * RECORDED Body text observed against a real instance, or carried over from + * `error-responses.ts`. + * PROVISIONAL Body text is a placeholder pending an answer from the + * Core/Enterprise team. Tests using these fixtures must assert on + * structure and on substrings the fixture itself defines + * (`DUPLICATED_TAG_KEY`), never on exact InfluxDB wording, so that + * correcting the fixture does not invalidate the assertion. + * + * Open questions that pin down the PROVISIONAL entries are tracked in the plan + * as A1 (503 on write_lp) and A2 (duplicate-tag-key response shape). See + * `tests/open-questions.stub.test.ts`. + */ + +import type { AxiosErrorShape, SdkErrorShape } from "./error-responses.js"; + +export type { AxiosErrorShape, SdkErrorShape }; + +// ── The 3.11 duplicate-tag-key case ───────────────────────────────────────── +// +// 3.11 rejects a line carrying the same tag key twice up front, the same way +// duplicate field keys were already refused. The impact map's central +// requirement is that the rejection reach the model naming the duplicated tag. + +/** + * Line protocol that 3.11 rejects: the same tag key appears twice. + * + * The plan writes this case as `m,t=a,t=a f=1i`. A one-letter tag key is not + * usable in an assertion — `not.toContain("t")` matches ordinary English in the + * generic error string and passes for the wrong reason — so the fixture uses a + * distinctive key instead. The condition under test is identical. + */ +export const DUPLICATE_TAG_LINE = "m,region=east,region=west f=1i"; + +/** The tag key duplicated in {@link DUPLICATE_TAG_LINE}. */ +export const DUPLICATED_TAG_KEY = "region"; + +/** + * PROVISIONAL (A2) — duplicated tag named under `data.error`. + * + * The shape the query-path resolver handles second. Matches how Core reports + * most write_lp rejections today. + */ +export const CORE_400_DUPLICATE_TAG_UNDER_ERROR: AxiosErrorShape = { + response: { + status: 400, + statusText: "Bad Request", + data: { + error: `invalid line protocol: duplicate tag key '${DUPLICATED_TAG_KEY}' on line 1`, + }, + }, + message: "Request failed with status code 400", +}; + +/** + * PROVISIONAL (A2) — duplicated tag named under `data.message`. + * + * The shape the query-path resolver handles first. Included because the write + * path must not regress the `{code, message}` family that the query path + * already had to grow support for. + */ +export const CORE_400_DUPLICATE_TAG_UNDER_MESSAGE: AxiosErrorShape = { + response: { + status: 400, + statusText: "Bad Request", + data: { + code: "invalid", + message: `invalid line protocol: duplicate tag key '${DUPLICATED_TAG_KEY}' on line 1`, + }, + }, + message: "Request failed with status code 400", +}; + +/** + * PROVISIONAL (A2) — duplicated tag named only inside the partial-write + * `data[].error_message` structure. + * + * This is the variant the query-path resolver does NOT reach: `data.error` + * resolves to the generic "partial write of line protocol occurred" and the + * actionable detail stays buried one level down. The MCP server sends + * `accept_partial=true` on the Core/Enterprise v3 path, so if 3.11 reports the + * rejection this way, resolving `data.error` is not sufficient. Tracked as A2 + * and A3. + */ +export const CORE_400_DUPLICATE_TAG_UNDER_PARTIAL_DATA: AxiosErrorShape = { + response: { + status: 400, + statusText: "Bad Request", + data: { + error: "partial write of line protocol occurred", + data: [ + { + error_message: `duplicate tag key '${DUPLICATED_TAG_KEY}'`, + line_number: 1, + original_line: DUPLICATE_TAG_LINE, + }, + ], + }, + }, + message: "Request failed with status code 400", +}; + +// ── Status arms that currently discard the body ───────────────────────────── + +/** RECORDED — Core rejects an unauthenticated request with this exact text. */ +export const CORE_401_UNAUTHENTICATED: AxiosErrorShape = { + response: { + status: 401, + statusText: "Unauthorized", + data: { error: "the request was not authenticated" }, + }, + message: "Request failed with status code 401", +}; + +/** PROVISIONAL — permission-scoped token refused by the write endpoint. */ +export const CORE_403_UNAUTHORIZED: AxiosErrorShape = { + response: { + status: 403, + statusText: "Forbidden", + data: { error: "the request was not authorized" }, + }, + message: "Request failed with status code 403", +}; + +/** PROVISIONAL — body exceeds the configured limit. Plain-text body. */ +export const CORE_413_PAYLOAD_TOO_LARGE: AxiosErrorShape = { + response: { + status: 413, + statusText: "Payload Too Large", + data: "the request body was too large: limit is 10485760 bytes", + }, + message: "Request failed with status code 413", +}; + +/** PROVISIONAL — line protocol parsed but semantically rejected. */ +export const CORE_422_UNPROCESSABLE: AxiosErrorShape = { + response: { + status: 422, + statusText: "Unprocessable Entity", + data: { + error: + "invalid column type for column 'value', expected iox::column_type::field::float, got iox::column_type::field::string", + }, + }, + message: "Request failed with status code 422", +}; + +// ── 503 — the status neither handler has an arm for ───────────────────────── + +/** + * PROVISIONAL (A1) — a stopped node. + * + * The 3.11 notes document the 400→503 change for `/api/v2/write`, which only + * the `clustered` product type uses. Whether `/api/v3/write_lp` behaves the + * same on Core/Enterprise is A1, and answerable only on a live instance. The + * fixture exists so the handler's 503 arm is testable either way — the arm is + * needed for the `clustered` path regardless of how A1 lands. + */ +export const CORE_503_NODE_STOPPED: AxiosErrorShape = { + response: { + status: 503, + statusText: "Service Unavailable", + data: { error: "node is stopped and not accepting writes" }, + }, + message: "Request failed with status code 503", +}; + +// ── Object-body fallback ──────────────────────────────────────────────────── + +/** + * RECORDED shape, unhandled status. + * + * Any status with no arm falls through to a fallback that interpolates + * `error.response.data` directly. When the body is parsed JSON — the normal + * case — the model receives `[object Object]`. + */ +export const CORE_500_OBJECT_BODY: AxiosErrorShape = { + response: { + status: 500, + statusText: "Internal Server Error", + data: { error: "internal error while persisting write" }, + }, + message: "Request failed with status code 500", +}; + +/** RECORDED shape — plain-text body on an unhandled status, which does survive. */ +export const CORE_500_STRING_BODY: AxiosErrorShape = { + response: { + status: 500, + statusText: "Internal Server Error", + data: "internal error while persisting write", + }, + message: "Request failed with status code 500", +}; + +// ── Cloud SDK (HttpError) write failures ──────────────────────────────────── +// +// `@influxdata/influxdb3-client` throws `HttpError`, which carries `statusCode` +// rather than `response.status`. Every status branch in the write handler tests +// `error.response?.status` and therefore misses on these entirely. + +/** PROVISIONAL — Cloud rejection of {@link DUPLICATE_TAG_LINE}. */ +export const CLOUD_SDK_400_DUPLICATE_TAG: SdkErrorShape = { + statusCode: 400, + statusMessage: "Bad Request", + body: `{"code":"invalid","message":"invalid line protocol: duplicate tag key '${DUPLICATED_TAG_KEY}' on line 1"}`, + json: { + code: "invalid", + message: `invalid line protocol: duplicate tag key '${DUPLICATED_TAG_KEY}' on line 1`, + }, + message: `invalid line protocol: duplicate tag key '${DUPLICATED_TAG_KEY}' on line 1`, +}; + +/** RECORDED shape — Cloud rejects an invalid token. */ +export const CLOUD_SDK_401_UNAUTHORIZED: SdkErrorShape = { + statusCode: 401, + statusMessage: "Unauthorized", + body: '{"code":"unauthorized","message":"unauthorized access"}', + json: { code: "unauthorized", message: "unauthorized access" }, + message: "unauthorized access", +}; + +/** PROVISIONAL — Cloud write rejected while a node is unavailable. */ +export const CLOUD_SDK_503_UNAVAILABLE: SdkErrorShape = { + statusCode: 503, + statusMessage: "Service Unavailable", + body: '{"code":"unavailable","message":"service temporarily unavailable"}', + json: { code: "unavailable", message: "service temporarily unavailable" }, + message: "service temporarily unavailable", +}; diff --git a/tests/helpers/write-service.ts b/tests/helpers/write-service.ts new file mode 100644 index 0000000..8af9b65 --- /dev/null +++ b/tests/helpers/write-service.ts @@ -0,0 +1,85 @@ +/** + * Stubs for exercising `WriteService` in isolation. + * + * `WriteService` reaches the network through exactly two seams on + * `BaseConnectionService` — `getInfluxHttpClient()` for the Core/Enterprise and + * Clustered HTTP paths, and `getClient()` for the Cloud SDK paths. Stubbing + * both makes every branch of `writeLineProtocol` and its error handler + * reachable without an InfluxDB instance. + */ + +import { vi } from "vitest"; +import { WriteService } from "../../src/services/write.service.js"; +import { BaseConnectionService } from "../../src/services/base-connection.service.js"; +import { InfluxProductType } from "../../src/helpers/enums/influx-product-types.enum.js"; + +export function stubBaseService( + type: InfluxProductType, +): BaseConnectionService { + return { + validateDataCapabilities: vi.fn(), + getConnectionInfo: vi.fn().mockReturnValue({ type }), + getInfluxHttpClient: vi.fn(), + getClient: vi.fn().mockReturnValue(null), + } as unknown as BaseConnectionService; +} + +/** An HTTP client whose `post` rejects with the given axios-shaped error. */ +export function httpClientThrowing(error: unknown) { + return { post: vi.fn().mockRejectedValue(error) }; +} + +/** An HTTP client whose `post` resolves, for asserting on the request made. */ +export function httpClientRecording() { + return { post: vi.fn().mockResolvedValue({ status: 204, data: "" }) }; +} + +/** An SDK client whose `write` rejects with the given `HttpError`-shaped error. */ +export function sdkClientThrowing(error: unknown) { + return { + write: vi.fn().mockRejectedValue(error), + queryPoints: vi.fn(), + close: vi.fn(), + }; +} + +/** An SDK client whose `write` resolves, for asserting on the call made. */ +export function sdkClientRecording() { + return { + write: vi.fn().mockResolvedValue(undefined), + queryPoints: vi.fn(), + close: vi.fn(), + }; +} + +/** + * Build a `WriteService` wired to a throwing HTTP client for the given product + * type, and return the message the model would see for a failed write. + */ +export async function writeErrorMessage( + type: InfluxProductType, + transport: { kind: "http" | "sdk"; error: unknown }, + lineProtocol = "m,t=a f=1i", + database = "mydb", +): Promise { + const base = stubBaseService(type); + if (transport.kind === "http") { + vi.mocked(base.getInfluxHttpClient).mockReturnValue( + httpClientThrowing(transport.error) as any, + ); + } else { + vi.mocked(base.getClient).mockReturnValue( + sdkClientThrowing(transport.error) as any, + ); + } + + const svc = new WriteService(base); + try { + await svc.writeLineProtocol(lineProtocol, database, { + precision: "nanosecond", + }); + } catch (error: any) { + return String(error.message); + } + throw new Error("expected writeLineProtocol to reject, but it resolved"); +} diff --git a/tests/packaging.test.ts b/tests/packaging.test.ts new file mode 100644 index 0000000..6f9c422 --- /dev/null +++ b/tests/packaging.test.ts @@ -0,0 +1,112 @@ +/** + * Packaging — patch item P7. + * + * `zod` is imported at runtime by `src/tools/index.ts` and by every + * `src/tools/categories/*.tools.ts`, but is declared in `devDependencies`. A + * clean `npm i --omit=dev` — or any consumer installing the published package + * — gets a server that cannot start. + * + * The check is written as an invariant over every runtime import rather than as + * a check on `zod` specifically, so it keeps working after P7 lands and catches + * the next occurrence. + */ + +import { describe, it, expect } from "vitest"; +import { readFileSync, readdirSync, statSync } from "node:fs"; +import { join, resolve } from "node:path"; +import { isBuiltin } from "node:module"; + +const ROOT = resolve(import.meta.dirname, ".."); + +const pkg = JSON.parse(readFileSync(join(ROOT, "package.json"), "utf8")); +const declaredRuntime = new Set(Object.keys(pkg.dependencies ?? {})); +const declaredDev = new Set(Object.keys(pkg.devDependencies ?? {})); + +function sourceFiles(dir: string): string[] { + return readdirSync(dir).flatMap((entry) => { + const full = join(dir, entry); + if (statSync(full).isDirectory()) return sourceFiles(full); + return full.endsWith(".ts") ? [full] : []; + }); +} + +/** `@scope/name/sub/path.js` → `@scope/name`; `name/sub` → `name`. */ +function packageNameOf(specifier: string): string { + const parts = specifier.split("/"); + return specifier.startsWith("@") ? parts.slice(0, 2).join("/") : parts[0]; +} + +/** Every bare package `src/` imports, excluding builtins and relative paths. */ +function runtimeImports(): Map { + const byPackage = new Map(); + + for (const file of sourceFiles(join(ROOT, "src"))) { + const source = readFileSync(file, "utf8"); + const specifiers = [ + ...source.matchAll(/(?:^|\n)\s*import\s[^;]*?from\s+["']([^"']+)["']/g), + ...source.matchAll(/\bimport\s*\(\s*["']([^"']+)["']\s*\)/g), + ].map((match) => match[1]); + + for (const specifier of specifiers) { + if (specifier.startsWith(".") || specifier.startsWith("/")) continue; + if (isBuiltin(specifier)) continue; + + const name = packageNameOf(specifier); + const files = byPackage.get(name) ?? []; + files.push(file.slice(ROOT.length + 1)); + byPackage.set(name, files); + } + } + + return byPackage; +} + +describe("runtime imports are declared as dependencies", () => { + const imports = runtimeImports(); + + it("finds the packages src/ imports at all (guards the scanner itself)", () => { + expect([...imports.keys()].sort()).toContain("@modelcontextprotocol/sdk"); + expect([...imports.keys()]).toContain("zod"); + }); + + it("every runtime import except zod is declared in dependencies", () => { + // Current state. When P7 moves zod, this test fails and should be replaced + // by the [P7] assertion below. + const missing = [...imports.keys()].filter((n) => !declaredRuntime.has(n)); + + expect(missing).toEqual(["zod"]); + }); + + it("zod is imported at runtime but declared only in devDependencies", () => { + const zodImporters = imports.get("zod") ?? []; + + expect(zodImporters.length).toBeGreaterThan(0); + expect(zodImporters).toContain("src/tools/index.ts"); + expect(declaredDev.has("zod")).toBe(true); + expect(declaredRuntime.has("zod")).toBe(false); + }); + + it("the published package ships build/, so the import survives to consumers", () => { + // Not a hypothetical: `files` includes `build`, and the compiled output + // keeps the bare `zod` specifier. The failure lands on the consumer. + expect(pkg.files).toContain("build"); + expect(pkg.main).toBe("./build/index.js"); + }); +}); + +describe.skip("[P7] zod is a runtime dependency", () => { + // Un-skip when P7 lands, and delete the two characterization tests above + // that assert the opposite. + const imports = runtimeImports(); + + it("no runtime import is missing from dependencies", () => { + const missing = [...imports.keys()].filter((n) => !declaredRuntime.has(n)); + + expect(missing).toEqual([]); + }); + + it("zod is not left duplicated in devDependencies", () => { + expect(declaredRuntime.has("zod")).toBe(true); + expect(declaredDev.has("zod")).toBe(false); + }); +}); diff --git a/tests/protocol-write.test.ts b/tests/protocol-write.test.ts new file mode 100644 index 0000000..ea823d5 --- /dev/null +++ b/tests/protocol-write.test.ts @@ -0,0 +1,227 @@ +/** + * Protocol-boundary tests for `write_line_protocol` and `health_check`. + * + * These run against the compiled server over stdio with an unreachable + * InfluxDB host — no instance is contacted for anything in this file. + */ + +import { describe, it, expect, beforeAll, afterAll } from "vitest"; +import { createTestClient, TestClient } from "./helpers/mcp-client.js"; + +/** + * Minimum viable environment per product type, per `validateConfig` + * (`src/config.ts`). The host is unreachable by design. + */ +const PRODUCT_ENVS: Record> = { + core: { + INFLUX_DB_PRODUCT_TYPE: "core", + INFLUX_DB_INSTANCE_URL: "http://localhost:19999/", + INFLUX_DB_TOKEN: "test-token-not-used", + }, + enterprise: { + INFLUX_DB_PRODUCT_TYPE: "enterprise", + INFLUX_DB_INSTANCE_URL: "http://localhost:19999/", + INFLUX_DB_TOKEN: "test-token-not-used", + }, +}; + +function textOf(result: unknown): string { + const content = (result as { content?: Array<{ text?: string }> }).content; + return content?.[0]?.text ?? ""; +} + +describe("write_line_protocol – advertised contract", () => { + let testClient: TestClient; + + beforeAll(async () => { + testClient = await createTestClient(PRODUCT_ENVS.core); + }); + + afterAll(async () => { + await testClient?.close(); + }); + + async function writeTool() { + const { tools } = await testClient.client.listTools(); + const tool = tools.find((t) => t.name === "write_line_protocol"); + expect(tool, "write_line_protocol must be advertised").toBeDefined(); + return tool!; + } + + it("requires database, data and precision", async () => { + const schema = (await writeTool()).inputSchema as any; + + expect(schema.required.sort()).toEqual(["data", "database", "precision"]); + expect(schema.additionalProperties).toBe(false); + }); + + it("advertises the four precision values the service maps", async () => { + // `mapPrecisionForCloud` in write.service.ts has an entry per value; an + // unmapped value would produce `undefined` in the Cloud/Clustered query + // string. Keeping the advertised enum and the map in step is what stops + // that. + const schema = (await writeTool()).inputSchema as any; + + expect(schema.properties.precision.enum).toEqual([ + "nanosecond", + "microsecond", + "millisecond", + "second", + ]); + }); + + it("advertises acceptPartial defaulting to true", async () => { + // The advertised default must match the service default, because the + // partial-write response shape depends on it. + const schema = (await writeTool()).inputSchema as any; + + expect(schema.properties.acceptPartial.default).toBe(true); + expect(schema.properties.noSync.default).toBe(false); + }); + + it("rejects an unknown precision at the validation boundary", async () => { + // zodSchema and inputSchema are maintained separately and must agree. + await expect( + testClient.client.callTool({ + name: "write_line_protocol", + arguments: { database: "mydb", data: "m f=1i", precision: "hour" }, + }), + ).rejects.toThrow(); + }); +}); + +describe("write failures reach the model as tool errors", () => { + let testClient: TestClient; + + beforeAll(async () => { + testClient = await createTestClient(PRODUCT_ENVS.core); + }); + + afterAll(async () => { + await testClient?.close(); + }); + + it("returns isError with non-empty text rather than throwing", async () => { + // The host is unreachable, so this exercises the transport-failure path: + // no `error.response`, so `handleWriteError` falls through to the fallback + // and interpolates `error.message`. + const result = await testClient.client.callTool({ + name: "write_line_protocol", + arguments: { + database: "mydb", + data: "m,t=a f=1i", + precision: "nanosecond", + }, + }); + + expect((result as { isError?: boolean }).isError).toBe(true); + expect(textOf(result)).not.toBe(""); + }); + + it("never renders an error body as [object Object]", async () => { + // The protocol-boundary half of this check. The unit-level half — where a + // parsed JSON body is what gets interpolated — is in write-error-core.test.ts. + const result = await testClient.client.callTool({ + name: "write_line_protocol", + arguments: { + database: "mydb", + data: "m,t=a f=1i", + precision: "nanosecond", + }, + }); + + expect(textOf(result)).not.toContain("[object Object]"); + }); + + it("names the database in the failure message", async () => { + const result = await testClient.client.callTool({ + name: "write_line_protocol", + arguments: { + database: "some_db", + data: "m,t=a f=1i", + precision: "nanosecond", + }, + }); + + expect(textOf(result)).toContain("some_db"); + }); +}); + +describe("capability detection – current state", () => { + let testClient: TestClient; + + beforeAll(async () => { + testClient = await createTestClient(PRODUCT_ENVS.core); + }); + + afterAll(async () => { + await testClient?.close(); + }); + + it("health_check reports HEALTHY against an unreachable instance", async () => { + // `hasAnySuccess` is set by `connectionInfo.isDataClientInitialized`, + // which is true whenever a client object was constructed — no request + // required. Both /ping and /health fail here, and the tool still reports + // success. + const result = await testClient.client.callTool({ + name: "health_check", + arguments: {}, + }); + + expect(textOf(result)).toContain("✅ HEALTHY"); + }); + + it("reports the configured product type verbatim, unverified", async () => { + // `INFLUX_DB_PRODUCT_TYPE` is trusted as given (`src/config.ts`); nothing + // checks it against the instance. Declaring `enterprise` against an + // unreachable host still reports `enterprise`. + const client = await createTestClient(PRODUCT_ENVS.enterprise); + try { + const result = await client.client.callTool({ + name: "health_check", + arguments: {}, + }); + expect(textOf(result)).toContain('"type": "enterprise"'); + } finally { + await client.close(); + } + }); + + it("reports no version when /ping is unreachable", async () => { + const result = await testClient.client.callTool({ + name: "health_check", + arguments: {}, + }); + const text = textOf(result); + + // `ping.version` is populated from the `x-influxdb-version` response + // header and passed straight through — never parsed, never compared + // against a minimum. + expect(text).toContain('"ok": false'); + expect(text).not.toContain('"version"'); + }); +}); + +describe.skip("health_check reflects reachability", () => { + // Un-skip when the health-reporting bug is fixed. A constructed client is + // not evidence the instance is reachable; only a successful /ping or + // /health is. + let testClient: TestClient; + + beforeAll(async () => { + testClient = await createTestClient(PRODUCT_ENVS.core); + }); + + afterAll(async () => { + await testClient?.close(); + }); + + it("reports FAILED when every endpoint check fails", async () => { + const result = await testClient.client.callTool({ + name: "health_check", + arguments: {}, + }); + + expect(textOf(result)).toContain("❌ FAILED"); + }); +}); diff --git a/tests/write-error-cloud.test.ts b/tests/write-error-cloud.test.ts new file mode 100644 index 0000000..0ed9d52 --- /dev/null +++ b/tests/write-error-cloud.test.ts @@ -0,0 +1,160 @@ +/** + * Write-path error fidelity — Cloud Dedicated / Cloud Serverless (SDK transport). + * + * Covers patch item P4 (normalize the cloud SDK error shape). + * + * `@influxdata/influxdb3-client` throws `HttpError`, which carries `statusCode` + * — not `error.response.status`. Every status branch in `handleWriteError` + * tests `error.response?.status`, so on these two product types no branch is + * ever reached and every failure lands in the fallback. + * + * Strictly outside the Core/Enterprise scope of the 3.11 patch, but it is the + * same function P1 and P2 rewrite: leaving it means those fixes only half-land. + * + * See `write-error-core.test.ts` for how the characterization / `[P*]` split + * works. + */ + +import { describe, it, expect } from "vitest"; +import { InfluxProductType } from "../src/helpers/enums/influx-product-types.enum.js"; +import { writeErrorMessage } from "./helpers/write-service.js"; +import { + CLOUD_SDK_400_DUPLICATE_TAG, + CLOUD_SDK_401_UNAUTHORIZED, + CLOUD_SDK_503_UNAVAILABLE, + DUPLICATE_TAG_LINE, + DUPLICATED_TAG_KEY, +} from "./fixtures/write-errors.js"; + +const CLOUD_TYPES = [ + ["cloud-dedicated", InfluxProductType.CloudDedicated], + ["cloud-serverless", InfluxProductType.CloudServerless], +] as const; + +const sdk = (error: unknown) => ({ kind: "sdk" as const, error }); + +describe("handleWriteError – Cloud SDK shape – current behavior", () => { + it.each(CLOUD_TYPES)( + "%s: a 400 never reaches the 400 arm", + async (_label, type) => { + const message = await writeErrorMessage( + type, + sdk(CLOUD_SDK_400_DUPLICATE_TAG), + DUPLICATE_TAG_LINE, + ); + + // The generic fallback, not the status arm. + expect(message).toMatch(/^Failed to write data to database 'mydb': /); + expect(message).not.toMatch(/^Bad request: /); + }, + ); + + it.each(CLOUD_TYPES)( + "%s: a 401 never reaches the 401 arm", + async (_label, type) => { + const message = await writeErrorMessage( + type, + sdk(CLOUD_SDK_401_UNAUTHORIZED), + ); + + expect(message).toMatch(/^Failed to write data to database 'mydb': /); + expect(message).not.toMatch(/^Unauthorized: /); + }, + ); + + it.each(CLOUD_TYPES)( + "%s: a 503 is not distinguishable as retryable", + async (_label, type) => { + const message = await writeErrorMessage( + type, + sdk(CLOUD_SDK_503_UNAVAILABLE), + ); + + expect(message).toMatch(/^Failed to write data to database 'mydb': /); + // The SDK's own message happens to say "temporarily unavailable", but + // nothing in the handler classifies it — a 503 and a 400 are rendered + // with the same prefix, which is the distinction P2 exists to restore. + expect(message).not.toMatch(/^Service unavailable/i); + }, + ); + + it("the SDK's message text does survive, unlike the axios path", async () => { + // Worth recording: on the cloud paths the fallback interpolates + // `error.message`, which HttpError populates from the response body. So + // the body is not lost here — it is the *classification* that is lost. + const message = await writeErrorMessage( + InfluxProductType.CloudServerless, + sdk(CLOUD_SDK_400_DUPLICATE_TAG), + DUPLICATE_TAG_LINE, + ); + + expect(message).toContain(DUPLICATED_TAG_KEY); + }); +}); + +describe.skip("[P4] cloud SDK errors are normalized before branching", () => { + // Un-skip when P4 lands. Expected: both `error.response.status` and + // `error.statusCode` resolve to one internal status, and both + // `error.response.data` and `error.body` / `error.json` resolve to one body, + // before any status branch runs. + + it.each(CLOUD_TYPES)( + "%s: a 400 reaches the 400 arm", + async (_label, type) => { + const message = await writeErrorMessage( + type, + sdk(CLOUD_SDK_400_DUPLICATE_TAG), + DUPLICATE_TAG_LINE, + ); + + expect(message).toMatch(/^Bad request: /); + expect(message).toContain(DUPLICATED_TAG_KEY); + }, + ); + + it.each(CLOUD_TYPES)( + "%s: a 401 reaches the 401 arm", + async (_label, type) => { + const message = await writeErrorMessage( + type, + sdk(CLOUD_SDK_401_UNAUTHORIZED), + ); + + expect(message).toMatch(/^Unauthorized: /); + expect(message).toContain("unauthorized access"); + }, + ); + + it.each(CLOUD_TYPES)( + "%s: a 503 reaches the retryable arm", + async (_label, type) => { + const message = await writeErrorMessage( + type, + sdk(CLOUD_SDK_503_UNAVAILABLE), + ); + + // Assert on the classification, not on body text: the SDK's own message + // for this fixture happens to read "temporarily unavailable", so a + // substring check alone would pass today for the wrong reason. + expect(message).not.toMatch(/^Failed to write data to database /); + expect(message).not.toMatch(/^Bad request: /); + expect(message).toMatch(/retry|temporar|again/i); + }, + ); + + it("resolves the body from HttpError.json, not only HttpError.message", async () => { + // `message` is a convenience the SDK derives; `json` / `body` are the + // response. Normalizing on the response keeps the two transports resolving + // the same field, which is the point of the shared helper P1 recommends. + const message = await writeErrorMessage( + InfluxProductType.CloudServerless, + sdk({ ...CLOUD_SDK_400_DUPLICATE_TAG, message: "Request failed" }), + DUPLICATE_TAG_LINE, + ); + + expect(message).toContain(DUPLICATED_TAG_KEY); + expect(message).not.toBe( + "Failed to write data to database 'mydb': Request failed", + ); + }); +}); diff --git a/tests/write-error-core.test.ts b/tests/write-error-core.test.ts new file mode 100644 index 0000000..7ffae43 --- /dev/null +++ b/tests/write-error-core.test.ts @@ -0,0 +1,246 @@ +/** + * Write-path error fidelity — Core / Enterprise (axios transport). + * + * Covers patch items P1 (preserve InfluxDB's error body) and P2 (503 arm, no + * `[object Object]`). Mirrors `query-error-core.test.ts`, which is the pattern + * the patch asks the write path to adopt. + * + * ── How to read this file ─────────────────────────────────────────────────── + * + * The `current behavior` blocks are active and passing. They pin down exactly + * what the model sees today, so the defect is described by an executable + * assertion rather than by prose. + * + * The `[P1]` / `[P2]` blocks are skipped. They are the acceptance criteria for + * the patch: un-skip them when implementing, and the paired characterization + * test above will go red at the same moment — that pairing is deliberate, it + * forces the stale characterization to be deleted rather than left behind. + */ + +import { describe, it, expect } from "vitest"; +import { InfluxProductType } from "../src/helpers/enums/influx-product-types.enum.js"; +import { writeErrorMessage } from "./helpers/write-service.js"; +import { + CORE_400_DUPLICATE_TAG_UNDER_ERROR, + CORE_400_DUPLICATE_TAG_UNDER_MESSAGE, + CORE_400_DUPLICATE_TAG_UNDER_PARTIAL_DATA, + CORE_401_UNAUTHENTICATED, + CORE_403_UNAUTHORIZED, + CORE_413_PAYLOAD_TOO_LARGE, + CORE_422_UNPROCESSABLE, + CORE_500_OBJECT_BODY, + CORE_500_STRING_BODY, + CORE_503_NODE_STOPPED, + DUPLICATE_TAG_LINE, + DUPLICATED_TAG_KEY, +} from "./fixtures/write-errors.js"; + +const http = (error: unknown) => ({ kind: "http" as const, error }); + +async function coreWriteError( + error: unknown, + lineProtocol?: string, +): Promise { + return writeErrorMessage(InfluxProductType.Core, http(error), lineProtocol); +} + +describe("handleWriteError – Core/Enterprise – current behavior", () => { + // Impact map 1.1: a duplicate-tag-key rejection must reach the model naming + // the tag. Today the 400 arm throws a fixed string and drops the body, so it + // does not — whatever shape InfluxDB reports the tag in. + it.each([ + ["data.error", CORE_400_DUPLICATE_TAG_UNDER_ERROR], + ["data.message", CORE_400_DUPLICATE_TAG_UNDER_MESSAGE], + ["data[].error_message", CORE_400_DUPLICATE_TAG_UNDER_PARTIAL_DATA], + ])( + "400: discards the duplicate-tag-key body reported under %s", + async (_shape, fixture) => { + const message = await coreWriteError(fixture, DUPLICATE_TAG_LINE); + + expect(message).toBe( + "Bad request: Invalid line protocol format or parameters", + ); + expect(message).not.toContain(DUPLICATED_TAG_KEY); + expect(message).not.toMatch(/duplicate/i); + }, + ); + + it("401: discards the InfluxDB body", async () => { + const message = await coreWriteError(CORE_401_UNAUTHENTICATED); + + expect(message).toBe("Unauthorized: Check your InfluxDB token permissions"); + expect(message).not.toContain("not authenticated"); + }); + + it("403: discards the InfluxDB body", async () => { + const message = await coreWriteError(CORE_403_UNAUTHORIZED); + + expect(message).toBe( + "Access denied: Insufficient permissions for database operations", + ); + expect(message).not.toContain("not authorized"); + }); + + it("413: discards the InfluxDB body, including the actual limit", async () => { + const message = await coreWriteError(CORE_413_PAYLOAD_TOO_LARGE); + + expect(message).toBe( + "Request entity too large: Reduce the size of your line protocol data", + ); + expect(message).not.toContain("10485760"); + }); + + it("422: discards the InfluxDB body, including the offending column", async () => { + const message = await coreWriteError(CORE_422_UNPROCESSABLE); + + expect(message).toBe("Unprocessable entity: Invalid line protocol syntax"); + expect(message).not.toContain("value"); + }); + + it("503: has no arm, so it falls through to the fallback", async () => { + const message = await coreWriteError(CORE_503_NODE_STOPPED); + + expect(message).toMatch(/^Failed to write data to database 'mydb':/); + // Nothing tells the model this is worth retrying. + expect(message).not.toMatch(/retry|temporar|again/i); + }); + + it("fallback renders a parsed JSON body as [object Object]", async () => { + const message = await coreWriteError(CORE_500_OBJECT_BODY); + + expect(message).toBe( + "Failed to write data to database 'mydb': [object Object]", + ); + expect(message).not.toContain("persisting write"); + }); + + // A no-regression guard, not a new requirement: this one already passes and + // must keep passing after P2 changes how bodies are rendered. + it("fallback does preserve a plain-text body", async () => { + // The one path that already works: `data` is a string, so interpolating it + // is lossless. Recorded so the P2 fix is not credited with more than it + // changes. + const message = await coreWriteError(CORE_500_STRING_BODY); + + expect(message).toBe( + "Failed to write data to database 'mydb': internal error while persisting write", + ); + }); + + it("applies identically on Enterprise", async () => { + const message = await writeErrorMessage( + InfluxProductType.Enterprise, + http(CORE_400_DUPLICATE_TAG_UNDER_ERROR), + DUPLICATE_TAG_LINE, + ); + + expect(message).toBe( + "Bad request: Invalid line protocol format or parameters", + ); + }); +}); + +// ── Acceptance criteria for the patch ─────────────────────────────────────── + +describe.skip("[P1] handleWriteError preserves the InfluxDB error body", () => { + // Un-skip when P1 lands. The expected resolution order is the one + // `handleQueryError` already uses: + // data.message → data.error → string body → statusText → error.message + + it("400: the duplicated tag key reaches the model (impact map 1.1)", async () => { + const message = await coreWriteError( + CORE_400_DUPLICATE_TAG_UNDER_ERROR, + DUPLICATE_TAG_LINE, + ); + + expect(message).toMatch(/^Bad request: /); + expect(message).toContain(DUPLICATED_TAG_KEY); + expect(message).toMatch(/duplicate/i); + }); + + it("400: resolves data.message ahead of data.error", async () => { + const message = await coreWriteError( + CORE_400_DUPLICATE_TAG_UNDER_MESSAGE, + DUPLICATE_TAG_LINE, + ); + + expect(message).toMatch(/^Bad request: /); + expect(message).toContain(DUPLICATED_TAG_KEY); + }); + + it("401: the InfluxDB body survives", async () => { + const message = await coreWriteError(CORE_401_UNAUTHENTICATED); + + expect(message).toMatch(/^Unauthorized: /); + expect(message).toContain("the request was not authenticated"); + }); + + it("403: the InfluxDB body survives", async () => { + const message = await coreWriteError(CORE_403_UNAUTHORIZED); + + expect(message).toMatch(/^Access denied: /); + expect(message).toContain("the request was not authorized"); + }); + + it("413: the actual size limit survives", async () => { + const message = await coreWriteError(CORE_413_PAYLOAD_TOO_LARGE); + + expect(message).toMatch(/^Request entity too large: /); + expect(message).toContain("10485760"); + }); + + it("422: the offending column survives", async () => { + const message = await coreWriteError(CORE_422_UNPROCESSABLE); + + expect(message).toMatch(/^Unprocessable entity: /); + expect(message).toContain("invalid column type for column 'value'"); + }); +}); + +describe.skip("[P2] handleWriteError adds a 503 arm and serializes bodies", () => { + it("503: is phrased as retryable and distinguishable from a bad request", async () => { + const message = await coreWriteError(CORE_503_NODE_STOPPED); + + expect(message).toMatch(/retry|temporar|again/i); + expect(message).not.toMatch(/^Bad request: /); + expect(message).toContain("node is stopped"); + }); + + it("fallback serializes a parsed JSON body instead of interpolating it", async () => { + const message = await coreWriteError(CORE_500_OBJECT_BODY); + + expect(message).not.toContain("[object Object]"); + expect(message).toContain("internal error while persisting write"); + }); + + // A no-regression guard, not a new requirement: cloud-serverless is already + // correct and must stay that way when the other four types change. + it("fallback still preserves a plain-text body unchanged", async () => { + const message = await coreWriteError(CORE_500_STRING_BODY); + + expect(message).toContain("internal error while persisting write"); + expect(message).not.toContain("[object Object]"); + }); +}); + +describe.skip("[P1/A2] partial-write bodies name the tag one level down", () => { + // Blocked on A2: whether 3.11 reports a duplicate-tag-key rejection inside + // the partial-write `data[].error_message` structure at all. If it does, + // resolving `data.error` is not enough — it yields the generic "partial write + // of line protocol occurred" and the actionable detail stays buried. The + // MCP server sends accept_partial=true on this path, so it would hit this. + // + // Un-skip only once A2 confirms the shape; delete if A2 rules it out. + it("400: the duplicated tag key reaches the model from data[].error_message", async () => { + const message = await coreWriteError( + CORE_400_DUPLICATE_TAG_UNDER_PARTIAL_DATA, + DUPLICATE_TAG_LINE, + ); + + expect(message).toContain(DUPLICATED_TAG_KEY); + expect(message).toMatch(/duplicate/i); + expect(message).not.toBe( + "Bad request: partial write of line protocol occurred", + ); + }); +}); diff --git a/tests/write-routing.test.ts b/tests/write-routing.test.ts new file mode 100644 index 0000000..5a4b222 --- /dev/null +++ b/tests/write-routing.test.ts @@ -0,0 +1,169 @@ +/** + * Write-path routing by product type. + * + * Pins the finding that settled impact map open item 3 ("which write endpoint + * does the server use?") and, with it, patch item P3. + * + * The 3.11 notes document the write 400→503 change for the **v2** write API. + * These tests establish that Core and Enterprise never touch v2 — they use + * `POST /api/v3/write_lp` — so the documented change does not reach them, and + * only `clustered` (a separate product on its own release train) is on the v2 + * path. What v3 returns for a stopped node is a different question, tracked as + * A1. + * + * These are regression tests, not acceptance criteria: they assert what the + * code does today and must keep doing. If one fails, the routing changed and + * the 3.11 impact analysis needs redoing. + */ + +import { describe, it, expect, vi } from "vitest"; +import { WriteService } from "../src/services/write.service.js"; +import { InfluxProductType } from "../src/helpers/enums/influx-product-types.enum.js"; +import { + stubBaseService, + httpClientRecording, + sdkClientRecording, +} from "./helpers/write-service.js"; + +const LINE = "m,t=a f=1i"; + +async function postFor(type: InfluxProductType, precision: any = "nanosecond") { + const base = stubBaseService(type); + const httpClient = httpClientRecording(); + vi.mocked(base.getInfluxHttpClient).mockReturnValue(httpClient as any); + + await new WriteService(base).writeLineProtocol(LINE, "mydb", { precision }); + + const [url, body, config] = httpClient.post.mock.calls[0]; + return { url: String(url), body, config, httpClient }; +} + +async function sdkWriteFor( + type: InfluxProductType, + precision: any = "nanosecond", +) { + const base = stubBaseService(type); + const client = sdkClientRecording(); + vi.mocked(base.getClient).mockReturnValue(client as any); + + await new WriteService(base).writeLineProtocol(LINE, "mydb", { precision }); + + return { call: client.write.mock.calls[0], client }; +} + +describe("write routing – Core and Enterprise use the v3 endpoint", () => { + it.each([ + ["core", InfluxProductType.Core], + ["enterprise", InfluxProductType.Enterprise], + ])("%s posts to /api/v3/write_lp, never /api/v2/write", async (_l, type) => { + const { url } = await postFor(type); + + expect(url).toMatch(/^\/api\/v3\/write_lp\?/); + expect(url).not.toContain("/api/v2/write"); + }); + + it("sends db, precision, accept_partial and no_sync as query parameters", async () => { + const { url } = await postFor(InfluxProductType.Core); + const params = new URLSearchParams(url.split("?")[1]); + + expect(params.get("db")).toBe("mydb"); + expect(params.get("precision")).toBe("nanosecond"); + expect(params.get("accept_partial")).toBe("true"); + expect(params.get("no_sync")).toBe("false"); + }); + + it("defaults accept_partial to true, so partial-write bodies are reachable", async () => { + // Relevant to A2/A3: because the server opts into partial writes, a + // rejection may arrive as a partial-write body rather than a whole-batch + // refusal. See the [P1/A2] block in write-error-core.test.ts. + const { url } = await postFor(InfluxProductType.Core); + + expect(new URLSearchParams(url.split("?")[1]).get("accept_partial")).toBe( + "true", + ); + }); + + it("uses the long-form precision name, unmapped", async () => { + const { url } = await postFor(InfluxProductType.Core, "millisecond"); + + expect(new URLSearchParams(url.split("?")[1]).get("precision")).toBe( + "millisecond", + ); + }); + + it("sends line protocol as a text/plain body", async () => { + const { body, config } = await postFor(InfluxProductType.Core); + + expect(body).toBe(LINE); + expect(config.headers["Content-Type"]).toMatch(/^text\/plain/); + }); +}); + +describe("write routing – Clustered is the only v2 caller", () => { + it("posts to /api/v2/write", async () => { + const { url } = await postFor(InfluxProductType.Clustered); + + expect(url).toMatch(/^\/api\/v2\/write\?/); + }); + + it("sends bucket (not db) and a short precision code", async () => { + const { url } = await postFor(InfluxProductType.Clustered, "millisecond"); + const params = new URLSearchParams(url.split("?")[1]); + + expect(params.get("bucket")).toBe("mydb"); + expect(params.get("db")).toBeNull(); + expect(params.get("precision")).toBe("ms"); + }); +}); + +describe("write routing – Cloud product types use the SDK client", () => { + it("cloud-dedicated calls client.write with the long-form precision", async () => { + const { call } = await sdkWriteFor( + InfluxProductType.CloudDedicated, + "millisecond", + ); + + expect(call[0]).toBe(LINE); + expect(call[1]).toBe("mydb"); + expect(call[3]).toEqual({ precision: "millisecond" }); + }); + + it("cloud-serverless calls client.write with a short precision code", async () => { + const { call } = await sdkWriteFor( + InfluxProductType.CloudServerless, + "millisecond", + ); + + expect(call[0]).toBe(LINE); + expect(call[1]).toBe("mydb"); + expect(call[3]).toEqual({ precision: "ms" }); + }); + + it.each([ + ["cloud-dedicated", InfluxProductType.CloudDedicated], + ["cloud-serverless", InfluxProductType.CloudServerless], + ])("%s makes no HTTP write request", async (_l, type) => { + const base = stubBaseService(type); + const httpClient = httpClientRecording(); + vi.mocked(base.getInfluxHttpClient).mockReturnValue(httpClient as any); + vi.mocked(base.getClient).mockReturnValue(sdkClientRecording() as any); + + await new WriteService(base).writeLineProtocol(LINE, "mydb", { + precision: "nanosecond", + }); + + expect(httpClient.post).not.toHaveBeenCalled(); + }); +}); + +describe("write routing – unknown product type", () => { + it("rejects rather than defaulting to a transport", async () => { + const base = stubBaseService("something-else" as InfluxProductType); + + await expect( + new WriteService(base).writeLineProtocol(LINE, "mydb", { + precision: "nanosecond", + }), + ).rejects.toThrow(/Unsupported InfluxDB product type/); + }); +}); From dcb767f157c3b1242c013fc35c36e27ff9892429 Mon Sep 17 00:00:00 2001 From: Jason Stirnaman Date: Tue, 28 Jul 2026 13:10:15 -0500 Subject: [PATCH 2/4] Correct duplicate-tag-key fixture to verified InfluxDB behavior Two of three guessed response shapes for a duplicate-tag-key write rejection never occur. Verified live against both Core (3.11.0-nightly) and Enterprise (3.11.0-0.rc.1, --upgrade-pacha-tree): a duplicate tag key is always reported under data[].error_message inside the partial-write body, byte-for-byte identical on both builds. Neither data.error nor data.message carries it. - Delete CORE_400_DUPLICATE_TAG_UNDER_ERROR and CORE_400_DUPLICATE_TAG_UNDER_MESSAGE; they don't represent real responses. - Update CORE_400_DUPLICATE_TAG_UNDER_PARTIAL_DATA with the actual wording ("invalid line protocol - multiple instances of 'region' tag found") and InfluxDB's own truncated original_line; promote from PROVISIONAL to RECORDED. - write-error-core.test.ts: drop the it.each over three shapes down to the one real shape; fix the /duplicate/i assertions in the [P1] acceptance block to the real wording; fold [P1/A2] into [P1] now that A2 is answered rather than leaving a separate blocked-on-A2 block. --- tests/fixtures/write-errors.ts | 75 ++++++++++----------------------- tests/write-error-core.test.ts | 77 +++++++++++----------------------- 2 files changed, 47 insertions(+), 105 deletions(-) diff --git a/tests/fixtures/write-errors.ts b/tests/fixtures/write-errors.ts index 2015886..be4a3ea 100644 --- a/tests/fixtures/write-errors.ts +++ b/tests/fixtures/write-errors.ts @@ -13,15 +13,20 @@ * * RECORDED Body text observed against a real instance, or carried over from * `error-responses.ts`. - * PROVISIONAL Body text is a placeholder pending an answer from the - * Core/Enterprise team. Tests using these fixtures must assert on - * structure and on substrings the fixture itself defines - * (`DUPLICATED_TAG_KEY`), never on exact InfluxDB wording, so that - * correcting the fixture does not invalidate the assertion. + * PROVISIONAL Body text is a placeholder pending live verification. Tests + * using these fixtures must assert on structure and on substrings + * the fixture itself defines (`DUPLICATED_TAG_KEY`), never on + * exact InfluxDB wording, so that correcting the fixture does not + * invalidate the assertion. * - * Open questions that pin down the PROVISIONAL entries are tracked in the plan - * as A1 (503 on write_lp) and A2 (duplicate-tag-key response shape). See - * `tests/open-questions.stub.test.ts`. + * A2 (duplicate-tag-key response shape) is resolved: verified 2026-07-28 + * against both an Enterprise instance (`--upgrade-pacha-tree`, 3.11.0-0.rc.1) + * and a Core instance (3.11.0-nightly) — identical response on both. See + * `CORE_400_DUPLICATE_TAG_UNDER_PARTIAL_DATA` below. + * + * A1 (503 on write_lp for a stopped node) remains open. See + * `tests/open-questions.stub.test.ts` on the `influxdb-3.11-compat-patch` + * branch. */ import type { AxiosErrorShape, SdkErrorShape } from "./error-responses.js"; @@ -48,51 +53,17 @@ export const DUPLICATE_TAG_LINE = "m,region=east,region=west f=1i"; export const DUPLICATED_TAG_KEY = "region"; /** - * PROVISIONAL (A2) — duplicated tag named under `data.error`. - * - * The shape the query-path resolver handles second. Matches how Core reports - * most write_lp rejections today. - */ -export const CORE_400_DUPLICATE_TAG_UNDER_ERROR: AxiosErrorShape = { - response: { - status: 400, - statusText: "Bad Request", - data: { - error: `invalid line protocol: duplicate tag key '${DUPLICATED_TAG_KEY}' on line 1`, - }, - }, - message: "Request failed with status code 400", -}; - -/** - * PROVISIONAL (A2) — duplicated tag named under `data.message`. - * - * The shape the query-path resolver handles first. Included because the write - * path must not regress the `{code, message}` family that the query path - * already had to grow support for. - */ -export const CORE_400_DUPLICATE_TAG_UNDER_MESSAGE: AxiosErrorShape = { - response: { - status: 400, - statusText: "Bad Request", - data: { - code: "invalid", - message: `invalid line protocol: duplicate tag key '${DUPLICATED_TAG_KEY}' on line 1`, - }, - }, - message: "Request failed with status code 400", -}; - -/** - * PROVISIONAL (A2) — duplicated tag named only inside the partial-write + * RECORDED (A2) — duplicated tag named only inside the partial-write * `data[].error_message` structure. * - * This is the variant the query-path resolver does NOT reach: `data.error` + * Verified 2026-07-28 against both Core (3.11.0-nightly) and Enterprise + * (3.11.0-0.rc.1, `--upgrade-pacha-tree`) — identical on both. `data.error` * resolves to the generic "partial write of line protocol occurred" and the - * actionable detail stays buried one level down. The MCP server sends - * `accept_partial=true` on the Core/Enterprise v3 path, so if 3.11 reports the - * rejection this way, resolving `data.error` is not sufficient. Tracked as A2 - * and A3. + * actionable detail is one level down, under `data[].error_message`. The MCP + * server sends `accept_partial=true` on the Core/Enterprise v3 path, so this + * is the shape a real rejection takes; resolving `data.error` alone is not + * sufficient. `original_line` is truncated by InfluxDB itself, mid tag value — + * not something our code does. */ export const CORE_400_DUPLICATE_TAG_UNDER_PARTIAL_DATA: AxiosErrorShape = { response: { @@ -102,9 +73,9 @@ export const CORE_400_DUPLICATE_TAG_UNDER_PARTIAL_DATA: AxiosErrorShape = { error: "partial write of line protocol occurred", data: [ { - error_message: `duplicate tag key '${DUPLICATED_TAG_KEY}'`, + error_message: `invalid line protocol - multiple instances of '${DUPLICATED_TAG_KEY}' tag found`, line_number: 1, - original_line: DUPLICATE_TAG_LINE, + original_line: "m,region=east,region", }, ], }, diff --git a/tests/write-error-core.test.ts b/tests/write-error-core.test.ts index 7ffae43..c01fb48 100644 --- a/tests/write-error-core.test.ts +++ b/tests/write-error-core.test.ts @@ -21,8 +21,6 @@ import { describe, it, expect } from "vitest"; import { InfluxProductType } from "../src/helpers/enums/influx-product-types.enum.js"; import { writeErrorMessage } from "./helpers/write-service.js"; import { - CORE_400_DUPLICATE_TAG_UNDER_ERROR, - CORE_400_DUPLICATE_TAG_UNDER_MESSAGE, CORE_400_DUPLICATE_TAG_UNDER_PARTIAL_DATA, CORE_401_UNAUTHENTICATED, CORE_403_UNAUTHORIZED, @@ -47,23 +45,20 @@ async function coreWriteError( describe("handleWriteError – Core/Enterprise – current behavior", () => { // Impact map 1.1: a duplicate-tag-key rejection must reach the model naming // the tag. Today the 400 arm throws a fixed string and drops the body, so it - // does not — whatever shape InfluxDB reports the tag in. - it.each([ - ["data.error", CORE_400_DUPLICATE_TAG_UNDER_ERROR], - ["data.message", CORE_400_DUPLICATE_TAG_UNDER_MESSAGE], - ["data[].error_message", CORE_400_DUPLICATE_TAG_UNDER_PARTIAL_DATA], - ])( - "400: discards the duplicate-tag-key body reported under %s", - async (_shape, fixture) => { - const message = await coreWriteError(fixture, DUPLICATE_TAG_LINE); - - expect(message).toBe( - "Bad request: Invalid line protocol format or parameters", - ); - expect(message).not.toContain(DUPLICATED_TAG_KEY); - expect(message).not.toMatch(/duplicate/i); - }, - ); + // does not. Verified shape (Core 3.11.0-nightly and Enterprise 3.11.0-0.rc.1, + // 2026-07-28): the tag name is nested under data[].error_message. + it("400: discards the duplicate-tag-key body reported under data[].error_message", async () => { + const message = await coreWriteError( + CORE_400_DUPLICATE_TAG_UNDER_PARTIAL_DATA, + DUPLICATE_TAG_LINE, + ); + + expect(message).toBe( + "Bad request: Invalid line protocol format or parameters", + ); + expect(message).not.toContain(DUPLICATED_TAG_KEY); + expect(message).not.toMatch(/multiple instances/i); + }); it("401: discards the InfluxDB body", async () => { const message = await coreWriteError(CORE_401_UNAUTHENTICATED); @@ -130,7 +125,7 @@ describe("handleWriteError – Core/Enterprise – current behavior", () => { it("applies identically on Enterprise", async () => { const message = await writeErrorMessage( InfluxProductType.Enterprise, - http(CORE_400_DUPLICATE_TAG_UNDER_ERROR), + http(CORE_400_DUPLICATE_TAG_UNDER_PARTIAL_DATA), DUPLICATE_TAG_LINE, ); @@ -147,25 +142,23 @@ describe.skip("[P1] handleWriteError preserves the InfluxDB error body", () => { // `handleQueryError` already uses: // data.message → data.error → string body → statusText → error.message + // Verified shape (Core 3.11.0-nightly and Enterprise 3.11.0-0.rc.1, + // 2026-07-28): the tag name is nested under data[].error_message, not + // data.error or data.message. Resolving data.error alone is not enough — + // it yields the generic "partial write of line protocol occurred" and the + // actionable detail stays buried one level down. it("400: the duplicated tag key reaches the model (impact map 1.1)", async () => { const message = await coreWriteError( - CORE_400_DUPLICATE_TAG_UNDER_ERROR, + CORE_400_DUPLICATE_TAG_UNDER_PARTIAL_DATA, DUPLICATE_TAG_LINE, ); expect(message).toMatch(/^Bad request: /); expect(message).toContain(DUPLICATED_TAG_KEY); - expect(message).toMatch(/duplicate/i); - }); - - it("400: resolves data.message ahead of data.error", async () => { - const message = await coreWriteError( - CORE_400_DUPLICATE_TAG_UNDER_MESSAGE, - DUPLICATE_TAG_LINE, + expect(message).toMatch(/multiple instances/i); + expect(message).not.toBe( + "Bad request: partial write of line protocol occurred", ); - - expect(message).toMatch(/^Bad request: /); - expect(message).toContain(DUPLICATED_TAG_KEY); }); it("401: the InfluxDB body survives", async () => { @@ -222,25 +215,3 @@ describe.skip("[P2] handleWriteError adds a 503 arm and serializes bodies", () = expect(message).not.toContain("[object Object]"); }); }); - -describe.skip("[P1/A2] partial-write bodies name the tag one level down", () => { - // Blocked on A2: whether 3.11 reports a duplicate-tag-key rejection inside - // the partial-write `data[].error_message` structure at all. If it does, - // resolving `data.error` is not enough — it yields the generic "partial write - // of line protocol occurred" and the actionable detail stays buried. The - // MCP server sends accept_partial=true on this path, so it would hit this. - // - // Un-skip only once A2 confirms the shape; delete if A2 rules it out. - it("400: the duplicated tag key reaches the model from data[].error_message", async () => { - const message = await coreWriteError( - CORE_400_DUPLICATE_TAG_UNDER_PARTIAL_DATA, - DUPLICATE_TAG_LINE, - ); - - expect(message).toContain(DUPLICATED_TAG_KEY); - expect(message).toMatch(/duplicate/i); - expect(message).not.toBe( - "Bad request: partial write of line protocol occurred", - ); - }); -}); From 0f43d69c47820e52cff624e20b4e6baf5ee9bb72 Mon Sep 17 00:00:00 2001 From: Jason Stirnaman Date: Tue, 28 Jul 2026 14:09:24 -0500 Subject: [PATCH 3/4] Fix CI failures: prettier formatting, serverless not-found wording - tests/README.md: table column alignment per prettier. - query-error-integration.test.ts asserted Core's "database not found" wording unconditionally. It was previously dead in CI (test:integration only matched integration.test.ts), so this never ran. Now that it runs, it also executes under the cloud-serverless CI job, which reports the same condition as "bucket ... not found" (v2-lineage vocabulary). Both wordings verified against real CI runs on this PR: Core job passed with "database not found", serverless job failed with the real "bucket ... not found" text. Pick the expected pattern by INFLUX_DB_PRODUCT_TYPE instead of hardcoding Core's wording. --- tests/README.md | 12 ++++++------ tests/query-error-integration.test.ts | 12 ++++++++++-- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/tests/README.md b/tests/README.md index b936bdc..67b2538 100644 --- a/tests/README.md +++ b/tests/README.md @@ -27,13 +27,13 @@ These appear in every vitest run, so the open list stays visible in CI output. ## Coverage for the write-path error and packaging fixes -| Bug | Tests | -| ----------------------------------------------- | --------------------------------------------------- | +| Bug | Tests | +| ----------------------------------------------- | ------------------------------------------------------- | | Write path drops the InfluxDB error body | `write-error-core.test.ts`, `write-error-cloud.test.ts` | -| Which endpoint each product type writes to | `write-routing.test.ts` | -| `zod` is a runtime dependency, declared as dev | `packaging.test.ts` | -| `ping()` version/build header passthrough | `base-connection-ping.test.ts` | -| `write_line_protocol` contract, error surfacing | `protocol-write.test.ts` | +| Which endpoint each product type writes to | `write-routing.test.ts` | +| `zod` is a runtime dependency, declared as dev | `packaging.test.ts` | +| `ping()` version/build header passthrough | `base-connection-ping.test.ts` | +| `write_line_protocol` contract, error surfacing | `protocol-write.test.ts` | None of these are version-gated — they're bugs in the current server, independent of which InfluxDB release is on the other end. diff --git a/tests/query-error-integration.test.ts b/tests/query-error-integration.test.ts index f3c09c2..a35f138 100644 --- a/tests/query-error-integration.test.ts +++ b/tests/query-error-integration.test.ts @@ -5,7 +5,15 @@ const RUN = process.env.INFLUX_TEST_ENABLED === "true" || process.env.INFLUX_TEST_ENABLED === "1"; -describe.skipIf(!RUN)("error path integration tests (live Core)", () => { +// Cloud Serverless reports a nonexistent database as a missing "bucket" (its +// v2-lineage vocabulary); Core/Enterprise say "database". Both verified +// against live CI runs, 2026-07-28. +const NOT_FOUND_PATTERN = + process.env.INFLUX_DB_PRODUCT_TYPE === "cloud-serverless" + ? /bucket .*not found/i + : /database not found/i; + +describe.skipIf(!RUN)("error path integration tests (live instance)", () => { let testClient: TestClient; function textContent(result: any): string { @@ -39,7 +47,7 @@ describe.skipIf(!RUN)("error path integration tests (live Core)", () => { const text = (result.content as Array<{ type: string; text: string }>)[0] ?.text; - expect(text).toMatch(/database not found/i); + expect(text).toMatch(NOT_FOUND_PATTERN); expect(text).not.toMatch("Internal Server Error"); }); From d9b5de37a07ccae79ab163b1761218362980d118 Mon Sep 17 00:00:00 2001 From: Jason Stirnaman Date: Tue, 28 Jul 2026 14:35:36 -0500 Subject: [PATCH 4/4] Remove dangling plan-item citations from test comments Comments referenced alphanumeric codes (P1-P7, A1-A4, "impact map 1.1") from a planning document that now lives only on the influxdb-3.11-compat-patch branch, not this one. A reader of this branch had no way to resolve what "P2" or "A2" meant. Replaced with plain, self-contained descriptions of the same technical content; renamed the matching describe.skip() block titles to drop the [P] tags. Also fixes a factual error in write-routing.test.ts and fixtures/write-errors.ts: the "InfluxDB 3.11 notes document a 400->503 change for /api/v2/write, which only the clustered product type uses" framing conflated two unrelated facts. The Core/Enterprise-scoped 3.11 release notes describe a legacy v1/v2 write endpoint that Core/ Enterprise themselves also expose for backward compatibility - not `InfluxProductType.Clustered`, a separate product (InfluxDB Clustered, grouped with Cloud Dedicated per src/config.ts:98-105) on its own release train that happens to also call /api/v2/write for unrelated reasons. This server never calls the legacy endpoint for Core/ Enterprise regardless; it always uses /api/v3/write_lp. --- tests/fixtures/write-errors.ts | 41 +++++++++++++++++++-------------- tests/packaging.test.ts | 18 +++++++-------- tests/write-error-cloud.test.ts | 28 ++++++++++++---------- tests/write-error-core.test.ts | 26 +++++++++++---------- tests/write-routing.test.ts | 30 ++++++++++++++---------- 5 files changed, 81 insertions(+), 62 deletions(-) diff --git a/tests/fixtures/write-errors.ts b/tests/fixtures/write-errors.ts index be4a3ea..bbb44aa 100644 --- a/tests/fixtures/write-errors.ts +++ b/tests/fixtures/write-errors.ts @@ -1,5 +1,5 @@ /** - * Write-path error fixtures for the InfluxDB 3.11 compatibility patch. + * Write-path error fixtures. * * Companion to `error-responses.ts`, which covers the query path. Shapes follow * the same two families: @@ -19,14 +19,14 @@ * exact InfluxDB wording, so that correcting the fixture does not * invalidate the assertion. * - * A2 (duplicate-tag-key response shape) is resolved: verified 2026-07-28 + * The duplicate-tag-key response shape is resolved: verified 2026-07-28 * against both an Enterprise instance (`--upgrade-pacha-tree`, 3.11.0-0.rc.1) * and a Core instance (3.11.0-nightly) — identical response on both. See * `CORE_400_DUPLICATE_TAG_UNDER_PARTIAL_DATA` below. * - * A1 (503 on write_lp for a stopped node) remains open. See - * `tests/open-questions.stub.test.ts` on the `influxdb-3.11-compat-patch` - * branch. + * Whether a stopped node makes `write_lp` return 503 (instead of the current + * 400) remains unverified — needs a live instance with a node stopped + * mid-write. See `CORE_503_NODE_STOPPED` below. */ import type { AxiosErrorShape, SdkErrorShape } from "./error-responses.js"; @@ -36,16 +36,16 @@ export type { AxiosErrorShape, SdkErrorShape }; // ── The 3.11 duplicate-tag-key case ───────────────────────────────────────── // // 3.11 rejects a line carrying the same tag key twice up front, the same way -// duplicate field keys were already refused. The impact map's central -// requirement is that the rejection reach the model naming the duplicated tag. +// duplicate field keys were already refused. The rejection needs to reach the +// model naming the duplicated tag, not just as a generic "bad request." /** * Line protocol that 3.11 rejects: the same tag key appears twice. * - * The plan writes this case as `m,t=a,t=a f=1i`. A one-letter tag key is not - * usable in an assertion — `not.toContain("t")` matches ordinary English in the - * generic error string and passes for the wrong reason — so the fixture uses a - * distinctive key instead. The condition under test is identical. + * A one-letter tag key (e.g. `m,t=a,t=a f=1i`) is not usable in an assertion — + * `not.toContain("t")` matches ordinary English in the generic error string + * and passes for the wrong reason — so the fixture uses a distinctive key + * instead. The condition under test is identical. */ export const DUPLICATE_TAG_LINE = "m,region=east,region=west f=1i"; @@ -53,7 +53,7 @@ export const DUPLICATE_TAG_LINE = "m,region=east,region=west f=1i"; export const DUPLICATED_TAG_KEY = "region"; /** - * RECORDED (A2) — duplicated tag named only inside the partial-write + * RECORDED — duplicated tag named only inside the partial-write * `data[].error_message` structure. * * Verified 2026-07-28 against both Core (3.11.0-nightly) and Enterprise @@ -131,13 +131,20 @@ export const CORE_422_UNPROCESSABLE: AxiosErrorShape = { // ── 503 — the status neither handler has an arm for ───────────────────────── /** - * PROVISIONAL (A1) — a stopped node. + * PROVISIONAL — a stopped node. * - * The 3.11 notes document the 400→503 change for `/api/v2/write`, which only - * the `clustered` product type uses. Whether `/api/v3/write_lp` behaves the - * same on Core/Enterprise is A1, and answerable only on a live instance. The + * InfluxDB 3.11's release notes (Core/Enterprise-scoped) document a 400→503 + * change for the legacy `/api/v2/write` endpoint that Core/Enterprise also + * expose for backward compatibility — not the `/api/v3/write_lp` endpoint + * this server actually calls for those product types. Whether + * `/api/v3/write_lp` behaves the same on a stopped node is unverified, + * answerable only on a live instance with a node stopped mid-write. The * fixture exists so the handler's 503 arm is testable either way — the arm is - * needed for the `clustered` path regardless of how A1 lands. + * also needed for the separate `Clustered` product (InfluxDB Clustered, the + * self-hosted sibling to Cloud Dedicated — not a multi-node Enterprise + * deployment), which does call `/api/v2/write` (see `write-routing.test.ts`). + * That's unrelated to these Core/Enterprise release notes: `Clustered` is a + * different product on its own release train. */ export const CORE_503_NODE_STOPPED: AxiosErrorShape = { response: { diff --git a/tests/packaging.test.ts b/tests/packaging.test.ts index 6f9c422..5d6a65e 100644 --- a/tests/packaging.test.ts +++ b/tests/packaging.test.ts @@ -1,14 +1,14 @@ /** - * Packaging — patch item P7. + * Packaging — `zod` is a misdeclared runtime dependency. * * `zod` is imported at runtime by `src/tools/index.ts` and by every * `src/tools/categories/*.tools.ts`, but is declared in `devDependencies`. A * clean `npm i --omit=dev` — or any consumer installing the published package * — gets a server that cannot start. * - * The check is written as an invariant over every runtime import rather than as - * a check on `zod` specifically, so it keeps working after P7 lands and catches - * the next occurrence. + * The check is written as an invariant over every runtime import rather than + * as a check on `zod` specifically, so it keeps working once `zod` moves to + * `dependencies` and catches the next occurrence. */ import { describe, it, expect } from "vitest"; @@ -70,8 +70,8 @@ describe("runtime imports are declared as dependencies", () => { }); it("every runtime import except zod is declared in dependencies", () => { - // Current state. When P7 moves zod, this test fails and should be replaced - // by the [P7] assertion below. + // Current state. When zod moves to dependencies, this test fails and + // should be replaced by the assertion below. const missing = [...imports.keys()].filter((n) => !declaredRuntime.has(n)); expect(missing).toEqual(["zod"]); @@ -94,9 +94,9 @@ describe("runtime imports are declared as dependencies", () => { }); }); -describe.skip("[P7] zod is a runtime dependency", () => { - // Un-skip when P7 lands, and delete the two characterization tests above - // that assert the opposite. +describe.skip("zod is a runtime dependency", () => { + // Un-skip when zod moves to dependencies, and delete the two + // characterization tests above that assert the opposite. const imports = runtimeImports(); it("no runtime import is missing from dependencies", () => { diff --git a/tests/write-error-cloud.test.ts b/tests/write-error-cloud.test.ts index 0ed9d52..a9865d0 100644 --- a/tests/write-error-cloud.test.ts +++ b/tests/write-error-cloud.test.ts @@ -1,18 +1,21 @@ /** * Write-path error fidelity — Cloud Dedicated / Cloud Serverless (SDK transport). * - * Covers patch item P4 (normalize the cloud SDK error shape). + * Covers normalizing the cloud SDK's error shape so it reaches the same + * status-handling code as the Core/Enterprise axios path. * * `@influxdata/influxdb3-client` throws `HttpError`, which carries `statusCode` * — not `error.response.status`. Every status branch in `handleWriteError` * tests `error.response?.status`, so on these two product types no branch is * ever reached and every failure lands in the fallback. * - * Strictly outside the Core/Enterprise scope of the 3.11 patch, but it is the - * same function P1 and P2 rewrite: leaving it means those fixes only half-land. + * `handleWriteError` is the same function that needs to preserve InfluxDB's + * error body and add a 503 arm for Core/Enterprise — fixing that without also + * normalizing this shape means the fix only half-lands, since cloud paths + * would still skip every status branch. * - * See `write-error-core.test.ts` for how the characterization / `[P*]` split - * works. + * See `write-error-core.test.ts` for how the "current behavior" vs. skipped + * acceptance-test split works. */ import { describe, it, expect } from "vitest"; @@ -73,7 +76,7 @@ describe("handleWriteError – Cloud SDK shape – current behavior", () => { expect(message).toMatch(/^Failed to write data to database 'mydb': /); // The SDK's own message happens to say "temporarily unavailable", but // nothing in the handler classifies it — a 503 and a 400 are rendered - // with the same prefix, which is the distinction P2 exists to restore. + // with the same prefix, and a 503 arm should distinguish them. expect(message).not.toMatch(/^Service unavailable/i); }, ); @@ -92,11 +95,11 @@ describe("handleWriteError – Cloud SDK shape – current behavior", () => { }); }); -describe.skip("[P4] cloud SDK errors are normalized before branching", () => { - // Un-skip when P4 lands. Expected: both `error.response.status` and - // `error.statusCode` resolve to one internal status, and both - // `error.response.data` and `error.body` / `error.json` resolve to one body, - // before any status branch runs. +describe.skip("cloud SDK errors are normalized before branching", () => { + // Un-skip when the cloud SDK error shape is normalized. Expected: both + // `error.response.status` and `error.statusCode` resolve to one internal + // status, and both `error.response.data` and `error.body` / `error.json` + // resolve to one body, before any status branch runs. it.each(CLOUD_TYPES)( "%s: a 400 reaches the 400 arm", @@ -145,7 +148,8 @@ describe.skip("[P4] cloud SDK errors are normalized before branching", () => { it("resolves the body from HttpError.json, not only HttpError.message", async () => { // `message` is a convenience the SDK derives; `json` / `body` are the // response. Normalizing on the response keeps the two transports resolving - // the same field, which is the point of the shared helper P1 recommends. + // the same field, which is the point of extracting a shared error-body + // resolver instead of letting the two handlers drift apart. const message = await writeErrorMessage( InfluxProductType.CloudServerless, sdk({ ...CLOUD_SDK_400_DUPLICATE_TAG, message: "Request failed" }), diff --git a/tests/write-error-core.test.ts b/tests/write-error-core.test.ts index c01fb48..6c3f4c9 100644 --- a/tests/write-error-core.test.ts +++ b/tests/write-error-core.test.ts @@ -1,9 +1,11 @@ /** * Write-path error fidelity — Core / Enterprise (axios transport). * - * Covers patch items P1 (preserve InfluxDB's error body) and P2 (503 arm, no - * `[object Object]`). Mirrors `query-error-core.test.ts`, which is the pattern - * the patch asks the write path to adopt. + * Covers two fixes needed in `handleWriteError`: preserving InfluxDB's error + * body instead of discarding it, and adding a 503 arm so retryable failures + * don't render as `[object Object]`. Mirrors `query-error-core.test.ts`, + * which already does this correctly on the query path — the pattern this + * file asks the write path to adopt. * * ── How to read this file ─────────────────────────────────────────────────── * @@ -11,8 +13,8 @@ * what the model sees today, so the defect is described by an executable * assertion rather than by prose. * - * The `[P1]` / `[P2]` blocks are skipped. They are the acceptance criteria for - * the patch: un-skip them when implementing, and the paired characterization + * The two `describe.skip(...)` blocks below are the acceptance criteria for + * the fix: un-skip them when implementing, and the paired characterization * test above will go red at the same moment — that pairing is deliberate, it * forces the stale characterization to be deleted rather than left behind. */ @@ -110,10 +112,10 @@ describe("handleWriteError – Core/Enterprise – current behavior", () => { }); // A no-regression guard, not a new requirement: this one already passes and - // must keep passing after P2 changes how bodies are rendered. + // must keep passing after the fallback's body-rendering changes. it("fallback does preserve a plain-text body", async () => { // The one path that already works: `data` is a string, so interpolating it - // is lossless. Recorded so the P2 fix is not credited with more than it + // is lossless. Recorded so the fix is not credited with more than it // changes. const message = await coreWriteError(CORE_500_STRING_BODY); @@ -135,10 +137,10 @@ describe("handleWriteError – Core/Enterprise – current behavior", () => { }); }); -// ── Acceptance criteria for the patch ─────────────────────────────────────── +// ── Acceptance criteria for the fix ───────────────────────────────────────── -describe.skip("[P1] handleWriteError preserves the InfluxDB error body", () => { - // Un-skip when P1 lands. The expected resolution order is the one +describe.skip("handleWriteError preserves the InfluxDB error body", () => { + // Un-skip when implemented. The expected resolution order is the one // `handleQueryError` already uses: // data.message → data.error → string body → statusText → error.message @@ -147,7 +149,7 @@ describe.skip("[P1] handleWriteError preserves the InfluxDB error body", () => { // data.error or data.message. Resolving data.error alone is not enough — // it yields the generic "partial write of line protocol occurred" and the // actionable detail stays buried one level down. - it("400: the duplicated tag key reaches the model (impact map 1.1)", async () => { + it("400: the duplicated tag key reaches the model", async () => { const message = await coreWriteError( CORE_400_DUPLICATE_TAG_UNDER_PARTIAL_DATA, DUPLICATE_TAG_LINE, @@ -190,7 +192,7 @@ describe.skip("[P1] handleWriteError preserves the InfluxDB error body", () => { }); }); -describe.skip("[P2] handleWriteError adds a 503 arm and serializes bodies", () => { +describe.skip("handleWriteError adds a 503 arm and serializes bodies", () => { it("503: is phrased as retryable and distinguishable from a bad request", async () => { const message = await coreWriteError(CORE_503_NODE_STOPPED); diff --git a/tests/write-routing.test.ts b/tests/write-routing.test.ts index 5a4b222..5e32b87 100644 --- a/tests/write-routing.test.ts +++ b/tests/write-routing.test.ts @@ -1,19 +1,24 @@ /** * Write-path routing by product type. * - * Pins the finding that settled impact map open item 3 ("which write endpoint - * does the server use?") and, with it, patch item P3. + * Establishes which endpoint and transport each product type uses to write. * - * The 3.11 notes document the write 400→503 change for the **v2** write API. - * These tests establish that Core and Enterprise never touch v2 — they use - * `POST /api/v3/write_lp` — so the documented change does not reach them, and - * only `clustered` (a separate product on its own release train) is on the v2 - * path. What v3 returns for a stopped node is a different question, tracked as - * A1. + * InfluxDB 3.11's release notes (Core/Enterprise-scoped) document a write + * 400→503 change for a stopped node, on the **legacy v1/v2 write endpoint** + * (`/api/v2/write`) that Core/Enterprise also expose for backward + * compatibility. This server never calls that endpoint for Core/Enterprise — + * it always uses `POST /api/v3/write_lp` — so the documented change does not + * reach this server's Core/Enterprise write path. The separate `Clustered` + * product (InfluxDB Clustered, the self-hosted sibling to Cloud Dedicated — + * not a multi-node Enterprise deployment) also posts to `/api/v2/write` + * below, but that's a separate, unrelated fact: `Clustered` is a different + * product on its own release train, not covered by these Core/Enterprise + * release notes at all. What v3 itself returns for a stopped node is + * untested here — it needs a live instance with a node stopped mid-write. * * These are regression tests, not acceptance criteria: they assert what the * code does today and must keep doing. If one fails, the routing changed and - * the 3.11 impact analysis needs redoing. + * needs re-auditing against the 3.11 release notes. */ import { describe, it, expect, vi } from "vitest"; @@ -73,9 +78,10 @@ describe("write routing – Core and Enterprise use the v3 endpoint", () => { }); it("defaults accept_partial to true, so partial-write bodies are reachable", async () => { - // Relevant to A2/A3: because the server opts into partial writes, a - // rejection may arrive as a partial-write body rather than a whole-batch - // refusal. See the [P1/A2] block in write-error-core.test.ts. + // Because the server opts into partial writes, a rejection may arrive as + // a partial-write body rather than a whole-batch refusal — see + // `CORE_400_DUPLICATE_TAG_UNDER_PARTIAL_DATA` in write-error-core.test.ts, + // which is exactly this shape. const { url } = await postFor(InfluxProductType.Core); expect(new URLSearchParams(url.split("?")[1]).get("accept_partial")).toBe(