Skip to content

Commit

Permalink
fix: cloud-sql-instance promise chaining (#245)
Browse files Browse the repository at this point in the history
Mixing async/await syntax with thenable method chaining was leading to a
promise chain that can potentially error with no handler, resulting in a
possible unhandled rejection.

This changeset fixes it by making sure a single promise chain is
returned in place.
  • Loading branch information
ruyadorno committed Nov 15, 2023
1 parent e1a50a5 commit 5fde5b3
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 11 deletions.
20 changes: 10 additions & 10 deletions src/cloud-sql-instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,18 +104,17 @@ export class CloudSQLInstance {
}) as ReturnType<typeof pThrottle>;
}

async forceRefresh(): Promise<void> {
forceRefresh(): Promise<RefreshResult> {
// if a refresh is already ongoing, just await for its promise to fulfill
// so that a new instance info is available before reconnecting
if (this.next) {
await this.next;
return;
return this.next;
}
this.cancelRefresh();
return this.refresh();
}

async refresh(): Promise<void> {
refresh(): Promise<RefreshResult> {
const currentRefreshId = this.scheduledRefreshID;

// Since forceRefresh might be invoked during an ongoing refresh
Expand Down Expand Up @@ -166,14 +165,15 @@ export class CloudSQLInstance {

// This refresh cycle has failed, releases ref to next
this.next = undefined;
})
// The rate limiter needs to be initialized _after_ assigning a ref
// to next in order to avoid race conditions with
// the forceRefresh check that ensures a refresh cycle is not ongoing
.then(() => {
this.initializeRateLimiter();
});

// The rate limiter needs to be initialized _after_ assigning a ref
// to next in order to avoid race conditions with
// the forceRefresh check that ensures a refresh cycle is not ongoing
await this.initializeRateLimiter();

await this.next;
return this.next as Promise<RefreshResult>;
}

// The performRefresh method will perform all the necessary async steps
Expand Down
2 changes: 1 addition & 1 deletion test/cloud-sql-instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ t.test('CloudSQLInstance', async t => {
limitRateInterval: 50,
});

t.rejects(
await t.rejects(
instance.refresh(),
/ERR/,
'should raise the specific error to the end user'
Expand Down

0 comments on commit 5fde5b3

Please sign in to comment.