Skip to content

Commit 8d8999d

Browse files
committed
fix: detect read-only errors on wrapped mysql2 errors
1 parent 6d0ba1b commit 8d8999d

3 files changed

Lines changed: 47 additions & 31 deletions

File tree

mysql/lib/mysql_error_handler.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,11 +71,9 @@ export class MySQLErrorHandler implements ErrorHandler {
7171
}
7272

7373
isReadOnlyConnectionError(e: Error): boolean {
74-
if (Object.prototype.hasOwnProperty.call(e, "errno")) {
75-
// @ts-ignore
76-
return MySQLErrorHandler.READ_ONLY_ERROR_CODES.includes(e["errno"]);
77-
}
78-
return false;
74+
// The driver error reaches the plugins wrapped in an AwsWrapperError, which keeps it as `cause`.
75+
const errno = (e as { errno?: number }).errno ?? (e.cause as { errno?: number })?.errno;
76+
return MySQLErrorHandler.READ_ONLY_ERROR_CODES.includes(errno);
7977
}
8078

8179
hasLoginError(): boolean {

pg/lib/abstract_pg_error_handler.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,9 @@ export abstract class AbstractPgErrorHandler implements ErrorHandler {
7474
}
7575

7676
isReadOnlyConnectionError(e: Error): boolean {
77-
if (Object.prototype.hasOwnProperty.call(e, "code")) {
78-
// @ts-ignore
79-
return AbstractPgErrorHandler.READ_ONLY_CONNECTION_SQLSTATE === e["code"];
80-
}
81-
return false;
77+
// pg hands the driver error over as-is today; the `cause` lookup covers it being wrapped later.
78+
const code = (e as { code?: string }).code ?? (e.cause as { code?: string })?.code;
79+
return code === AbstractPgErrorHandler.READ_ONLY_CONNECTION_SQLSTATE;
8280
}
8381

8482
hasLoginError(): boolean {

tests/unit/error_handler.test.ts

Lines changed: 41 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,40 +16,60 @@
1616

1717
import { MySQLErrorHandler } from "../../mysql/lib/mysql_error_handler";
1818
import { PgErrorHandler } from "../../pg/lib/pg_error_handler";
19+
import { AwsWrapperError } from "../../common/lib/utils/errors";
1920

2021
function errorWith(props: Record<string, any>): Error {
2122
return Object.assign(new Error("test"), props);
2223
}
2324

25+
function asAwsWrapperError(error: Error): Error {
26+
return new AwsWrapperError(error.message, error);
27+
}
28+
2429
describe("test read only connection error", () => {
25-
const pgHandler = new PgErrorHandler();
26-
const mysqlHandler = new MySQLErrorHandler();
30+
// mysql2 errors reach the plugins wrapped by ClientUtils, keeping the driver error as `cause`.
31+
describe("mysql", () => {
32+
const handler = new MySQLErrorHandler();
2733

28-
it("test pg read only detected by sqlstate 25006", () => {
29-
expect(pgHandler.isReadOnlyConnectionError(errorWith({ code: "25006" }))).toBe(true);
30-
});
34+
it("test read only errno 1290 detected", () => {
35+
expect(handler.isReadOnlyConnectionError(asAwsWrapperError(errorWith({ errno: 1290 })))).toBe(true);
36+
});
3137

32-
it("test pg unrelated sqlstate not detected", () => {
33-
expect(pgHandler.isReadOnlyConnectionError(errorWith({ code: "42601" }))).toBe(false);
34-
});
38+
it("test read only errno 1836 detected", () => {
39+
expect(handler.isReadOnlyConnectionError(asAwsWrapperError(errorWith({ errno: 1836 })))).toBe(true);
40+
});
3541

36-
it("test pg error without code not detected", () => {
37-
expect(pgHandler.isReadOnlyConnectionError(new Error("cannot execute in a read-only transaction"))).toBe(false);
38-
});
42+
it("test unrelated errno not detected", () => {
43+
expect(handler.isReadOnlyConnectionError(asAwsWrapperError(errorWith({ errno: 1064 })))).toBe(false);
44+
});
3945

40-
it("test mysql read only detected by errno 1290", () => {
41-
expect(mysqlHandler.isReadOnlyConnectionError(errorWith({ errno: 1290 }))).toBe(true);
42-
});
46+
it("test error without errno not detected", () => {
47+
expect(handler.isReadOnlyConnectionError(asAwsWrapperError(new Error("read only")))).toBe(false);
48+
});
4349

44-
it("test mysql read only detected by errno 1836", () => {
45-
expect(mysqlHandler.isReadOnlyConnectionError(errorWith({ errno: 1836 }))).toBe(true);
50+
it("test read only errno detected on an unwrapped error", () => {
51+
expect(handler.isReadOnlyConnectionError(errorWith({ errno: 1290 }))).toBe(true);
52+
});
4653
});
4754

48-
it("test mysql unrelated errno not detected", () => {
49-
expect(mysqlHandler.isReadOnlyConnectionError(errorWith({ errno: 1064 }))).toBe(false);
50-
});
55+
// pg hands the driver error to the plugins as-is.
56+
describe("pg", () => {
57+
const handler = new PgErrorHandler();
58+
59+
it("test read only sqlstate detected", () => {
60+
expect(handler.isReadOnlyConnectionError(errorWith({ code: "25006" }))).toBe(true);
61+
});
62+
63+
it("test unrelated sqlstate not detected", () => {
64+
expect(handler.isReadOnlyConnectionError(errorWith({ code: "42601" }))).toBe(false);
65+
});
66+
67+
it("test error without sqlstate not detected", () => {
68+
expect(handler.isReadOnlyConnectionError(new Error("cannot execute INSERT in a read-only transaction"))).toBe(false);
69+
});
5170

52-
it("test mysql error without errno not detected", () => {
53-
expect(mysqlHandler.isReadOnlyConnectionError(new Error("read only"))).toBe(false);
71+
it("test read only sqlstate detected on a wrapped error", () => {
72+
expect(handler.isReadOnlyConnectionError(asAwsWrapperError(errorWith({ code: "25006" })))).toBe(true);
73+
});
5474
});
5575
});

0 commit comments

Comments
 (0)