fix: trigger failover on read-only connection errors in strict-writer mode

common/lib/error_handler.ts

@@ -23,6 +23,9 @@ export interface ErrorHandler {
 
   isSyntaxError(e: Error): boolean;
 
+  // True when a write was attempted on a read-only connection, e.g. a writer demoted after failover.
+  isReadOnlyConnectionError(e: Error): boolean;
+
   /**
    * Checks whether there has been an unexpected error emitted and if the error is a type of login error.
    */

common/lib/plugin_service.ts

@@ -702,6 +702,10 @@ export class PluginServiceImpl implements PluginService, HostListProviderService
     return this.getDialect().getErrorHandler().isSyntaxError(e);
   }
 
+  isReadOnlyConnectionError(e: Error): boolean {
+    return this.getDialect().getErrorHandler().isReadOnlyConnectionError(e);
+  }
+
   hasLoginError(): boolean {
     return this.getDialect().getErrorHandler().hasLoginError();
   }

common/lib/plugins/failover/failover_plugin.ts

@@ -522,7 +522,13 @@ export class FailoverPlugin extends AbstractConnectionPlugin {
     }
 
     if (error instanceof Error) {
-      return this.pluginService.isNetworkError(error);
+      if (this.pluginService.isNetworkError(error)) {
+        return true;
+      }
+      // A demoted writer returns read-only errors on writes, which must trigger failover.
+      if (this.failoverMode === FailoverMode.STRICT_WRITER) {
+        return this.pluginService.isReadOnlyConnectionError(error);
+      }
     }
 
     return false;

common/lib/plugins/failover2/failover2_plugin.ts

@@ -475,7 +475,13 @@ export class Failover2Plugin extends AbstractConnectionPlugin implements CanRele
     }
 
     if (error instanceof Error) {
-      return this.pluginService.isNetworkError(error);
+      if (this.pluginService.isNetworkError(error)) {
+        return true;
+      }
+      // A demoted writer returns read-only errors on writes, which must trigger failover.
+      if (this.failoverMode === FailoverMode.STRICT_WRITER) {
+        return this.pluginService.isReadOnlyConnectionError(error);
+      }
     }
 
     return false;

mysql/lib/mysql_error_handler.ts

@@ -24,6 +24,7 @@ export class MySQLErrorHandler implements ErrorHandler {
   private unexpectedError: Error | null = null;
   protected static readonly SYNTAX_ERROR_CODES = ["42000", "42S02"];
   protected static readonly SYNTAX_ERROR_MESSAGE = "You have an error in your SQL syntax";
+  protected static readonly READ_ONLY_ERROR_CODES = [1290, 1836];
   protected isNoOpListenerAttached = false;
   protected isTrackingListenerAttached = false;
 
@@ -69,6 +70,14 @@ export class MySQLErrorHandler implements ErrorHandler {
     return e.message.includes(MySQLErrorHandler.SYNTAX_ERROR_MESSAGE);
   }
 
+  isReadOnlyConnectionError(e: Error): boolean {
+    if (Object.prototype.hasOwnProperty.call(e, "errno")) {
+      // @ts-ignore
+      return MySQLErrorHandler.READ_ONLY_ERROR_CODES.includes(e["errno"]);
+    }
+    return false;
+  }
+
   hasLoginError(): boolean {
     return this.unexpectedError !== null && this.isLoginError(this.unexpectedError);
   }

pg/lib/abstract_pg_error_handler.ts

@@ -23,6 +23,7 @@ export abstract class AbstractPgErrorHandler implements ErrorHandler {
   protected unexpectedError: Error | null = null;
   protected static readonly SYNTAX_ERROR_CODE = "42601";
   protected static readonly SYNTAX_ERROR_MESSAGE = "syntax error";
+  protected static readonly READ_ONLY_CONNECTION_SQLSTATE = "25006";
   protected isNoOpListenerAttached = false;
   protected isTrackingListenerAttached = false;
 
@@ -72,6 +73,14 @@ export abstract class AbstractPgErrorHandler implements ErrorHandler {
     return e.message.includes(AbstractPgErrorHandler.SYNTAX_ERROR_MESSAGE);
   }
 
+  isReadOnlyConnectionError(e: Error): boolean {
+    if (Object.prototype.hasOwnProperty.call(e, "code")) {
+      // @ts-ignore
+      return AbstractPgErrorHandler.READ_ONLY_CONNECTION_SQLSTATE === e["code"];
+    }
+    return false;
+  }
+
   hasLoginError(): boolean {
     return this.unexpectedError !== null && this.isLoginError(this.unexpectedError);
   }

tests/unit/error_handler.test.ts

@@ -0,0 +1,55 @@
+/*
+  Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ 
+  Licensed under the Apache License, Version 2.0 (the "License").
+  You may not use this file except in compliance with the License.
+  You may obtain a copy of the License at
+ 
+  http://www.apache.org/licenses/LICENSE-2.0
+ 
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+*/
+
+import { MySQLErrorHandler } from "../../mysql/lib/mysql_error_handler";
+import { PgErrorHandler } from "../../pg/lib/pg_error_handler";
+
+function errorWith(props: Record<string, any>): Error {
+  return Object.assign(new Error("test"), props);
+}
+
+describe("test read only connection error", () => {
+  const pgHandler = new PgErrorHandler();
+  const mysqlHandler = new MySQLErrorHandler();
+
+  it("test pg read only detected by sqlstate 25006", () => {
+    expect(pgHandler.isReadOnlyConnectionError(errorWith({ code: "25006" }))).toBe(true);
+  });
+
+  it("test pg unrelated sqlstate not detected", () => {
+    expect(pgHandler.isReadOnlyConnectionError(errorWith({ code: "42601" }))).toBe(false);
+  });
+
+  it("test pg error without code not detected", () => {
+    expect(pgHandler.isReadOnlyConnectionError(new Error("cannot execute in a read-only transaction"))).toBe(false);
+  });
+
+  it("test mysql read only detected by errno 1290", () => {
+    expect(mysqlHandler.isReadOnlyConnectionError(errorWith({ errno: 1290 }))).toBe(true);
+  });
+
+  it("test mysql read only detected by errno 1836", () => {
+    expect(mysqlHandler.isReadOnlyConnectionError(errorWith({ errno: 1836 }))).toBe(true);
+  });
+
+  it("test mysql unrelated errno not detected", () => {
+    expect(mysqlHandler.isReadOnlyConnectionError(errorWith({ errno: 1064 }))).toBe(false);
+  });
+
+  it("test mysql error without errno not detected", () => {
+    expect(mysqlHandler.isReadOnlyConnectionError(new Error("read only"))).toBe(false);
+  });
+});

tests/unit/failover2_plugin.test.ts

@@ -325,4 +325,56 @@ describe("reader failover handler", () => {
     expect(count).toStrictEqual(2);
     verify(mockRdsHostListProvider.getRdsUrlType()).never();
   });
+
+  it("test execute - read-only error triggers failover in strict-writer mode", async () => {
+    properties.set(WrapperProperties.ENABLE_CLUSTER_AWARE_FAILOVER.name, true);
+    when(mockPluginService.getAllHosts()).thenReturn([builder.withHost("hostA").build()]);
+    when(mockPluginService.getCurrentHostInfo()).thenReturn(null);
+    when(mockPluginService.isNetworkError(anything())).thenReturn(false);
+    when(mockPluginService.isReadOnlyConnectionError(anything())).thenReturn(true);
+
+    initializePlugin(instance(mockPluginService));
+    const spyPlugin: Failover2Plugin = spy(plugin);
+    when(spyPlugin.failover()).thenResolve();
+    plugin.failoverMode = FailoverMode.STRICT_WRITER;
+
+    const readOnlyError = new Error("read only");
+    await expect(plugin.execute("query", () => Promise.reject(readOnlyError))).rejects.toBe(readOnlyError);
+
+    verify(spyPlugin.failover()).once();
+  });
+
+  it("test execute - read-only error does not trigger failover outside strict-writer mode", async () => {
+    properties.set(WrapperProperties.ENABLE_CLUSTER_AWARE_FAILOVER.name, true);
+    when(mockPluginService.getAllHosts()).thenReturn([builder.withHost("hostA").build()]);
+    when(mockPluginService.isNetworkError(anything())).thenReturn(false);
+    when(mockPluginService.isReadOnlyConnectionError(anything())).thenReturn(true);
+
+    initializePlugin(instance(mockPluginService));
+    const spyPlugin: Failover2Plugin = spy(plugin);
+    when(spyPlugin.failover()).thenResolve();
+    plugin.failoverMode = FailoverMode.READER_OR_WRITER;
+
+    const readOnlyError = new Error("read only");
+    await expect(plugin.execute("query", () => Promise.reject(readOnlyError))).rejects.toBe(readOnlyError);
+
+    verify(spyPlugin.failover()).never();
+  });
+
+  it("test execute - network error triggers failover regardless of mode", async () => {
+    properties.set(WrapperProperties.ENABLE_CLUSTER_AWARE_FAILOVER.name, true);
+    when(mockPluginService.getAllHosts()).thenReturn([builder.withHost("hostA").build()]);
+    when(mockPluginService.getCurrentHostInfo()).thenReturn(null);
+    when(mockPluginService.isNetworkError(anything())).thenReturn(true);
+
+    initializePlugin(instance(mockPluginService));
+    const spyPlugin: Failover2Plugin = spy(plugin);
+    when(spyPlugin.failover()).thenResolve();
+    plugin.failoverMode = FailoverMode.READER_OR_WRITER;
+
+    const networkError = new Error("network");
+    await expect(plugin.execute("query", () => Promise.reject(networkError))).rejects.toBe(networkError);
+
+    verify(spyPlugin.failover()).once();
+  });
 });

tests/unit/failover_plugin.test.ts

@@ -430,4 +430,56 @@ describe("reader failover handler", () => {
     expect(count).toStrictEqual(2);
     verify(mockRdsHostListProvider.getRdsUrlType()).never();
   });
+
+  it("test execute - read-only error triggers failover in strict-writer mode", async () => {
+    properties.set(WrapperProperties.ENABLE_CLUSTER_AWARE_FAILOVER.name, true);
+    when(mockPluginService.getAllHosts()).thenReturn([builder.withHost("hostA").build()]);
+    when(mockPluginService.getCurrentHostInfo()).thenReturn(null);
+    when(mockPluginService.isNetworkError(anything())).thenReturn(false);
+    when(mockPluginService.isReadOnlyConnectionError(anything())).thenReturn(true);
+
+    initializePlugin(instance(mockPluginService));
+    const spyPlugin: FailoverPlugin = spy(plugin);
+    when(spyPlugin.pickNewConnection()).thenResolve();
+    plugin.failoverMode = FailoverMode.STRICT_WRITER;
+
+    const readOnlyError = new Error("read only");
+    await expect(plugin.execute("query", () => Promise.reject(readOnlyError))).rejects.toBe(readOnlyError);
+
+    verify(spyPlugin.pickNewConnection()).once();
+  });
+
+  it("test execute - read-only error does not trigger failover outside strict-writer mode", async () => {
+    properties.set(WrapperProperties.ENABLE_CLUSTER_AWARE_FAILOVER.name, true);
+    when(mockPluginService.getAllHosts()).thenReturn([builder.withHost("hostA").build()]);
+    when(mockPluginService.isNetworkError(anything())).thenReturn(false);
+    when(mockPluginService.isReadOnlyConnectionError(anything())).thenReturn(true);
+
+    initializePlugin(instance(mockPluginService));
+    const spyPlugin: FailoverPlugin = spy(plugin);
+    when(spyPlugin.pickNewConnection()).thenResolve();
+    plugin.failoverMode = FailoverMode.READER_OR_WRITER;
+
+    const readOnlyError = new Error("read only");
+    await expect(plugin.execute("query", () => Promise.reject(readOnlyError))).rejects.toBe(readOnlyError);
+
+    verify(spyPlugin.pickNewConnection()).never();
+  });
+
+  it("test execute - network error triggers failover regardless of mode", async () => {
+    properties.set(WrapperProperties.ENABLE_CLUSTER_AWARE_FAILOVER.name, true);
+    when(mockPluginService.getAllHosts()).thenReturn([builder.withHost("hostA").build()]);
+    when(mockPluginService.getCurrentHostInfo()).thenReturn(null);
+    when(mockPluginService.isNetworkError(anything())).thenReturn(true);
+
+    initializePlugin(instance(mockPluginService));
+    const spyPlugin: FailoverPlugin = spy(plugin);
+    when(spyPlugin.pickNewConnection()).thenResolve();
+    plugin.failoverMode = FailoverMode.READER_OR_WRITER;
+
+    const networkError = new Error("network");
+    await expect(plugin.execute("query", () => Promise.reject(networkError))).rejects.toBe(networkError);
+
+    verify(spyPlugin.pickNewConnection()).once();
+  });
 });