Skip to content
This repository was archived by the owner on Mar 4, 2026. It is now read-only.
This repository was archived by the owner on Mar 4, 2026. It is now read-only.

Database.runTransactionAsync should check that the error returned can be re-tried instead of the indefinite while(true) loop #2166

Closed
Closed
Database.runTransactionAsync should check that the error returned can be re-tried instead of the indefinite while(true) loop#2166
Assignees
surbhigarg92
Labels
api: spannerIssues related to the googleapis/nodejs-spanner API.priority: p2Moderately-important priority. Fix may not be included in next release.type: bugError or flaw in code with unintended results or allowing sub-optimal usage patterns.
@odeke-em

Description

@odeke-em
odeke-em
opened on Oct 21, 2024

If we examine the code for Database.runTransactionAsync, it invokes a while(true) loop retrying on any error without differentiating if the error can be retried or not and the only way to break out of that loop is by throwing an exception

nodejs-spanner/src/database.ts

Lines 3370 to 3406 in 2a19ef1

while (true) {
try {
const [session, transaction] = await promisify(getSession)();
transaction.requestOptions = Object.assign(
transaction.requestOptions || {},
options.requestOptions
);
if (options.optimisticLock) {
transaction.useOptimisticLock();
}
if (options.excludeTxnFromChangeStreams) {
transaction.excludeTxnFromChangeStreams();
}
sessionId = session?.id;
span.addEvent('Using Session', {'session.id': sessionId});
const runner = new AsyncTransactionRunner<T>(
session,
transaction,
runFn,
options
);
try {
return await runner.run();
} finally {
this.pool_.release(session);
}
} catch (e) {
if (!isSessionNotFoundError(e as ServiceError)) {
span.addEvent('No session available', {
'session.id': sessionId,
});
throw e;
}
}
}
}

The only way I can test out functionality of that code in error is by throwing an exception and there aren't tests for what happens with a variety of errors. Let's please add them.

Metadata

Metadata

Assignees

Labels

api: spannerIssues related to the googleapis/nodejs-spanner API.priority: p2Moderately-important priority. Fix may not be included in next release.type: bugError or flaw in code with unintended results or allowing sub-optimal usage patterns.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions