fix: Remove Begin transaction on non retryable error

src/transaction.ts

@@ -758,21 +758,6 @@ export class Snapshot extends EventEmitter {
             this._update(response.metadata!.transaction);
           }
         })
-        .on('error', err => {
-          setSpanError(span, err);
-          const wasAborted = isErrorAborted(err);
-          if (!this.id && this._useInRunner && !wasAborted) {
-            // TODO: resolve https://github.com/googleapis/nodejs-spanner/issues/2170
-            this.begin();
-          } else {
-            if (wasAborted) {
-              span.addEvent('Stream broken. Not safe to retry', {
-                'transaction.id': this.id?.toString(),
-              });
-            }
-          }
-          span.end();
-        })
         .on('end', err => {
           if (err) {
             setSpanError(span, err);
@@ -1349,22 +1334,6 @@ export class Snapshot extends EventEmitter {
             this._update(response.metadata!.transaction);
           }
         })
-        .on('error', err => {
-          setSpanError(span, err as Error);
-          const wasAborted = isErrorAborted(err);
-          if (!this.id && this._useInRunner && !wasAborted) {
-            span.addEvent('Stream broken. Safe to retry');
-            // TODO: resolve https://github.com/googleapis/nodejs-spanner/issues/2170
-            this.begin();
-          } else {
-            if (wasAborted) {
-              span.addEvent('Stream broken. Not safe to retry', {
-                'transaction.id': this.id?.toString(),
-              });
-            }
-          }
-          span.end();
-        })
         .on('end', err => {
           if (err) {
             setSpanError(span, err as Error);

test/spanner.ts

@@ -452,11 +452,8 @@ describe('Spanner with mock server', () => {
         request.requestOptions!.transactionTag,
         'transaction-tag'
       );
-      const beginTxnRequest = spannerMock.getRequests().find(val => {
-        return (val as v1.BeginTransactionRequest).options?.readWrite;
-      }) as v1.BeginTransactionRequest;
       assert.strictEqual(
-        beginTxnRequest.options?.readWrite!.readLockMode,
+        request.transaction?.begin?.readWrite?.readLockMode,
         'OPTIMISTIC'
       );
     });
@@ -3758,120 +3755,6 @@ describe('Spanner with mock server', () => {
       );
     });
 
-    it('should use beginTransaction on retry for unknown reason', async () => {
-      const database = newTestDatabase();
-      await database.runTransactionAsync(async tx => {
-        try {
-          await tx.runUpdate(invalidSql);
-          assert.fail('missing expected error');
-        } catch (e) {
-          assert.strictEqual(
-            (e as ServiceError).message,
-            `${grpc.status.NOT_FOUND} NOT_FOUND: ${fooNotFoundErr.message}`
-          );
-        }
-        await tx.run(selectSql);
-        await tx.commit();
-      });
-      await database.close();
-
-      const beginTxnRequest = spannerMock
-        .getRequests()
-        .filter(val => (val as v1.BeginTransactionRequest).options?.readWrite)
-        .map(req => req as v1.BeginTransactionRequest);
-      assert.deepStrictEqual(beginTxnRequest.length, 1);
-    });
-
-    it('should use beginTransaction on retry for unknown reason with excludeTxnFromChangeStreams', async () => {
-      const database = newTestDatabase();
-      await database.runTransactionAsync(
-        {
-          excludeTxnFromChangeStreams: true,
-        },
-        async tx => {
-          try {
-            await tx.runUpdate(invalidSql);
-            assert.fail('missing expected error');
-          } catch (e) {
-            assert.strictEqual(
-              (e as ServiceError).message,
-              `${grpc.status.NOT_FOUND} NOT_FOUND: ${fooNotFoundErr.message}`
-            );
-          }
-          await tx.run(selectSql);
-          await tx.commit();
-        }
-      );
-      await database.close();
-
-      const beginTxnRequest = spannerMock
-        .getRequests()
-        .filter(val => (val as v1.BeginTransactionRequest).options?.readWrite)
-        .map(req => req as v1.BeginTransactionRequest);
-      assert.deepStrictEqual(beginTxnRequest.length, 1);
-      assert.strictEqual(
-        beginTxnRequest[0].options?.excludeTxnFromChangeStreams,
-        true
-      );
-    });
-
-    it('should use beginTransaction for streaming on retry for unknown reason', async () => {
-      const database = newTestDatabase();
-      await database.runTransactionAsync(async tx => {
-        try {
-          await getRowCountFromStreamingSql(tx!, {sql: invalidSql});
-          assert.fail('missing expected error');
-        } catch (e) {
-          assert.strictEqual(
-            (e as ServiceError).message,
-            `${grpc.status.NOT_FOUND} NOT_FOUND: ${fooNotFoundErr.message}`
-          );
-        }
-        await tx.run(selectSql);
-        await tx.commit();
-      });
-      await database.close();
-
-      const beginTxnRequest = spannerMock
-        .getRequests()
-        .filter(val => (val as v1.BeginTransactionRequest).options?.readWrite)
-        .map(req => req as v1.BeginTransactionRequest);
-      assert.deepStrictEqual(beginTxnRequest.length, 1);
-    });
-
-    it('should use beginTransaction for streaming on retry for unknown reason with excludeTxnFromChangeStreams', async () => {
-      const database = newTestDatabase();
-      await database.runTransactionAsync(
-        {
-          excludeTxnFromChangeStreams: true,
-        },
-        async tx => {
-          try {
-            await getRowCountFromStreamingSql(tx!, {sql: invalidSql});
-            assert.fail('missing expected error');
-          } catch (e) {
-            assert.strictEqual(
-              (e as ServiceError).message,
-              `${grpc.status.NOT_FOUND} NOT_FOUND: ${fooNotFoundErr.message}`
-            );
-          }
-          await tx.run(selectSql);
-          await tx.commit();
-        }
-      );
-      await database.close();
-
-      const beginTxnRequest = spannerMock
-        .getRequests()
-        .filter(val => (val as v1.BeginTransactionRequest).options?.readWrite)
-        .map(req => req as v1.BeginTransactionRequest);
-      assert.deepStrictEqual(beginTxnRequest.length, 1);
-      assert.strictEqual(
-        beginTxnRequest[0].options?.excludeTxnFromChangeStreams,
-        true
-      );
-    });
-
     it('should fail if beginTransaction fails', async () => {
       const database = newTestDatabase();
       const err = {