fix: rollback with no id (#2231)

Fixes #2103

observability-test/transaction.ts

@@ -501,14 +501,14 @@ describe('Transaction', () => {
       transaction.id = ID;
     });
 
-    it('error with unset `id`', done => {
+    it('no error with unset `id`', done => {
       const expectedError = new Error(
         'Transaction ID is unknown, nothing to rollback.'
       );
       delete transaction.id;
 
       transaction.rollback(err => {
-        assert.deepStrictEqual(err, expectedError);
+        assert.deepStrictEqual(err, null);
 
         const exportResults = extractExportedSpans();
         const actualSpanNames = exportResults.spanNames;
@@ -521,7 +521,9 @@ describe('Transaction', () => {
           `span names mismatch:\n\tGot:  ${actualSpanNames}\n\tWant: ${expectedSpanNames}`
         );
 
-        const expectedEventNames = [];
+        const expectedEventNames = [
+          'Transaction ID is unknown, nothing to rollback.',
+        ];
         assert.deepStrictEqual(
           actualEventNames,
           expectedEventNames,
@@ -532,12 +534,12 @@ describe('Transaction', () => {
         const spans = exportResults.spans;
         const firstSpan = spans[0];
         assert.strictEqual(
-          SpanStatusCode.ERROR,
+          SpanStatusCode.UNSET,
           firstSpan.status.code,
-          'Unexpected an span status code'
+          'Unexpected span status code'
         );
         assert.strictEqual(
-          expectedError.message,
+          undefined,
           firstSpan.status.message,
           'Unexpected span status message'
         );

src/session-pool.ts

@@ -783,11 +783,7 @@ export class SessionPool extends EventEmitter implements SessionPoolInterface {
       return;
     }
 
-    try {
-      await this._createSessions(needed);
-    } catch (e) {
-      this.emit('error', e);
-    }
+    await this._createSessions(needed);
   }
 
   /**
@@ -993,7 +989,24 @@ export class SessionPool extends EventEmitter implements SessionPoolInterface {
     const pings = sessions.map(session => this._ping(session));
 
     await Promise.all(pings);
-    return this._fill();
+    try {
+      await this._fill();
+    } catch (error) {
+      // Ignore `Database not found` error. This allows a user to call instance.database('db-name')
+      // for a database that does not yet exist with SessionPoolOptions.min > 0.
+      const err = error as ServiceError;
+      if (
+        isDatabaseNotFoundError(err) ||
+        isInstanceNotFoundError(err) ||
+        isCreateSessionPermissionError(err) ||
+        isDefaultCredentialsNotSetError(err) ||
+        isProjectIdNotSetInEnvironmentError(err)
+      ) {
+        return;
+      }
+      this.emit('error', err);
+    }
+    return;
   }
 
   /**

src/transaction.ts

@@ -2517,12 +2517,9 @@ export class Transaction extends Dml {
     };
     return startTrace('Transaction.rollback', traceConfig, span => {
       if (!this.id) {
-        const err = new Error(
-          'Transaction ID is unknown, nothing to rollback.'
-        ) as ServiceError;
-        setSpanError(span, err);
+        span.addEvent('Transaction ID is unknown, nothing to rollback.');
         span.end();
-        callback!(err);
+        callback(null);
         return;
       }
 

test/session-pool.ts

@@ -953,12 +953,10 @@ describe('SessionPool', () => {
 
       stub.rejects(error);
 
-      sessionPool.once('error', err => {
+      sessionPool._fill().catch(err => {
         assert.strictEqual(err, error);
         done();
       });
-
-      sessionPool._fill();
     });
   });
 
@@ -1286,6 +1284,23 @@ describe('SessionPool', () => {
 
       assert.strictEqual(fillStub.callCount, 1);
     });
+
+    it('should not throw error when database not found', async () => {
+      const fakeSessions = [createSession()];
+      sandbox.stub(sessionPool, '_getIdleSessions').returns(fakeSessions);
+
+      const error = {
+        code: grpc.status.NOT_FOUND,
+        message: 'Database not found',
+      } as grpc.ServiceError;
+      sandbox.stub(sessionPool, '_fill').rejects(error);
+
+      try {
+        await sessionPool._pingIdleSessions();
+      } catch (err) {
+        assert.ifError(err);
+      }
+    });
   });
 
   describe('_release', () => {

test/transaction.ts

@@ -1975,15 +1975,10 @@ describe('Transaction', () => {
         transaction.id = ID;
       });
 
-      it('should return an error if the `id` is not set', done => {
-        const expectedError = new Error(
-          'Transaction ID is unknown, nothing to rollback.'
-        );
-
+      it('should not return an error if the `id` is not set', done => {
         delete transaction.id;
-
         transaction.rollback(err => {
-          assert.deepStrictEqual(err, expectedError);
+          assert.deepStrictEqual(err, null);
           done();
         });
       });