Skip to content

Commit 66ce012

Browse files
committed
Simplify close-ordering fix
Tighten the previous commit to the minimal diff: guard the existing unregisterContext call instead of restructuring the function. Restores the unchanged comment on TelemetryClient.unregisterContext. Co-authored-by: Isaac
1 parent 65273f1 commit 66ce012

2 files changed

Lines changed: 16 additions & 27 deletions

File tree

lib/telemetry/TelemetryClient.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -169,10 +169,8 @@ class TelemetryClient implements IClientContext {
169169

170170
/**
171171
* Remove a `DBSQLClient`'s context from the pool. Called by
172-
* `TelemetryClientProvider.releaseClient` in the multi-refcount case so the
173-
* exporter doesn't keep trying to use a closing context. Deliberately
174-
* NOT called on the last refcount release: `close()` needs the snapshot
175-
* pair to resolve auth/connection providers for the final flush.
172+
* `TelemetryClientProvider.releaseClient` before refcount decrement so the
173+
* exporter doesn't keep trying to use a closed context.
176174
*
177175
* Uses the cached snapshot pair (`context`, `authProvider`) from register
178176
* time, not a fresh `context.getAuthProvider?.()` call. If the underlying

lib/telemetry/TelemetryClientProvider.ts

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -133,35 +133,26 @@ class TelemetryClientProvider {
133133
return;
134134
}
135135

136+
// Skip unregister on the last release so close()'s final flush can still
137+
// resolve auth/connection providers from the FIFO snapshot.
136138
if (holder.refCount > 1) {
137-
// Other registrants remain — drop this context now so subsequent
138-
// flushes by surviving consumers don't try to authenticate via a
139-
// context whose underlying DBSQLClient is closing.
140139
holder.client.unregisterContext(context);
141-
holder.refCount -= 1;
142-
logger.log(LogLevel.debug, `TelemetryClient reference count for ${host}: ${holder.refCount}`);
143-
return;
144140
}
145-
146-
// Last refcount holder. Keep the context registered through close()
147-
// so the final flush can still resolve `getAuthProvider()` and
148-
// `getConnectionProvider()` from the FIFO snapshot — otherwise the
149-
// exporter drops the final batch with "missing Authorization header".
150-
// The TelemetryClient is fully closed below, so the lingering FIFO
151-
// entry has no further effect.
152141
holder.refCount -= 1;
153142
logger.log(LogLevel.debug, `TelemetryClient reference count for ${host}: ${holder.refCount}`);
154143

155-
// Remove from map BEFORE awaiting close so a concurrent
156-
// getOrCreateClient creates a fresh instance rather than receiving
157-
// this closing one.
158-
this.clients.delete(key);
159-
try {
160-
await holder.client.close();
161-
logger.log(LogLevel.debug, `Closed and removed TelemetryClient for host: ${host}`);
162-
} catch (error) {
163-
const msg = error instanceof Error ? error.message : String(error);
164-
logger.log(LogLevel.debug, `Error releasing TelemetryClient: ${msg}`);
144+
if (holder.refCount <= 0) {
145+
// Remove from map BEFORE awaiting close so a concurrent
146+
// getOrCreateClient creates a fresh instance rather than receiving
147+
// this closing one.
148+
this.clients.delete(key);
149+
try {
150+
await holder.client.close();
151+
logger.log(LogLevel.debug, `Closed and removed TelemetryClient for host: ${host}`);
152+
} catch (error) {
153+
const msg = error instanceof Error ? error.message : String(error);
154+
logger.log(LogLevel.debug, `Error releasing TelemetryClient: ${msg}`);
155+
}
165156
}
166157
}
167158

0 commit comments

Comments
 (0)