Skip to content

Commit 9da98a3

Browse files
committed
fix(clickhouse): make the patched rawMessage non-enumerable at the source
Making the field non-enumerable only on InsertError left the leak open one layer down: the client logs the raw ClickHouseError at every insert-error site before converting it, and the patch assigned rawMessage as a plain enumerable property, so the offending-row snippet still reached structured logs. The patch now defines the field non-enumerable itself. Covers the patch's behaviour with a test that reads the row hint back through the public parseError export, which also fails loudly if the patch ever stops being applied.
1 parent 36df1a1 commit 9da98a3

3 files changed

Lines changed: 25 additions & 7 deletions

File tree

internal-packages/clickhouse/src/client/errors.test.ts

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,28 @@
1+
import { ClickHouseError, parseError } from "@clickhouse/client";
12
import { describe, expect, it } from "vitest";
23
import { InsertError } from "./errors.js";
34

4-
describe("InsertError.rawMessage", () => {
5-
const rawMessage =
6-
'Code: 117. DB::Exception: Cannot parse JSON object here: {"secret":"customer-payload"}: (at row 3)';
5+
const rawMessage =
6+
'Code: 117. DB::Exception: Cannot parse JSON object here: {"secret":"customer-payload"}: (at row 3) (INCORRECT_DATA) (version 26.2.1.1)';
7+
8+
describe("patched ClickHouseError.rawMessage", () => {
9+
it("carries the untruncated text so the recovery path can read the row hint", () => {
10+
const error = parseError(rawMessage);
11+
12+
expect(error).toBeInstanceOf(ClickHouseError);
13+
expect((error as ClickHouseError).rawMessage).toContain("at row 3");
14+
});
715

16+
it("stays out of anything that serializes own enumerable properties", () => {
17+
const error = parseError(rawMessage);
18+
19+
expect(Object.keys(error)).not.toContain("rawMessage");
20+
expect(JSON.stringify(error)).not.toContain("customer-payload");
21+
expect(JSON.stringify({ ...error })).not.toContain("customer-payload");
22+
});
23+
});
24+
25+
describe("InsertError.rawMessage", () => {
826
it("is readable by the recovery path", () => {
927
expect(new InsertError("Cannot parse JSON object here", { rawMessage }).rawMessage).toBe(
1028
rawMessage

patches/@clickhouse__client-common@1.12.1.patch

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ index e06fa4ab36537c2a448857c1b001e70cd771bfe5..9750bd7e1285ac8cf2c8095b592ed344
2121
- return new ClickHouseError(groups);
2222
+ const error = new ClickHouseError(groups);
2323
+ if (!inputIsError && groups.message.startsWith("Cannot parse JSON object")) {
24-
+ error.rawMessage = message;
24+
+ Object.defineProperty(error, "rawMessage", { value: message, enumerable: false });
2525
+ }
2626
+ return error;
2727
}

pnpm-lock.yaml

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)