Skip to content

fix: Firestore Client caching stub in bad state issue #2365

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jun 19, 2025
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions dev/src/v1/firestore_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ export class FirestoreClient {
private _defaults: {[method: string]: gax.CallSettings};
private _universeDomain: string;
private _servicePath: string;
private _stubFailed = false;
auth: gax.GoogleAuth;
descriptors: Descriptors = {
page: {},
Expand Down Expand Up @@ -289,10 +290,13 @@ export class FirestoreClient {
*/
initialize() {
// If the client stub promise is already initialized, return immediately.
if (this.firestoreStub) {
if (this.firestoreStub && !this._stubFailed) {
return this.firestoreStub;
}

// Reset _stubFailed because we are re-attempting create
this._stubFailed = false;

// Put together the "service stub" for
// google.firestore.v1.Firestore.
this.firestoreStub = this._gaxGrpc.createStub(
Expand Down Expand Up @@ -328,8 +332,8 @@ export class FirestoreClient {
];
for (const methodName of firestoreStubMethods) {
const callPromise = this.firestoreStub.then(
stub =>
(...args: Array<{}>) => {
stub => {
return (...args: Array<{}>) => {
if (this._terminated) {
if (methodName in this.descriptors.stream) {
const stream = new PassThrough({objectMode: true});
Expand All @@ -347,9 +351,13 @@ export class FirestoreClient {
}
const func = stub[methodName];
return func.apply(stub, args);
},
(err: Error | null | undefined) => () => {
throw err;
};
},
(err: Error | null | undefined) => {
this._stubFailed = true;
return () => {
throw err;
};
}
);

Expand Down
48 changes: 48 additions & 0 deletions dev/test/gapic_firestore_v1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import * as firestoreModule from '../src/v1';
import {PassThrough} from 'stream';

import {protobuf, LocationProtos} from 'google-gax';
import {expect} from 'chai';

// Dynamically loaded proto JSON is needed to get the type information
// to fill in default values for request objects
Expand Down Expand Up @@ -3173,4 +3174,51 @@ describe('v1.FirestoreClient', () => {
);
});
});

describe('initialize', () => {
it('retries on error', async () => {
const client = new firestoreModule.FirestoreClient({
credentials: {client_email: 'bogus', private_key: 'bogus'},
projectId: 'bogus',
});
const createStubStub = sinon.stub();
createStubStub.returns(Promise.reject('error'));

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore -- typing not required for testing
client['_gaxGrpc'] = {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore -- typing not required for testing
createStub: createStubStub,
};

try {
await client.initialize();
} catch (e: unknown) {
expect(e).to.equal('error');
}

try {
await client.initialize();
} catch (e: unknown) {
expect(e).to.equal('error');
}

expect(createStubStub.callCount).to.equal(2);
});

it('does not recreate stub when one exists', async () => {
const client = new firestoreModule.FirestoreClient({
credentials: {client_email: 'bogus', private_key: 'bogus'},
projectId: 'bogus',
});

const createStubSpy = sinon.spy(client['_gaxGrpc'], 'createStub');

await client.initialize();
await client.initialize();

expect(createStubSpy.callCount).to.equal(1);
});
});
});