Skip to content

Commit 04ad0a4

Browse files
wu-huigcf-owl-bot[bot]MarkDuckworth
authored
fix: Firestore Client caching stub in bad state issue (#2365)
* [Fix] Fix Firestore Client caching stub in bad state issue * fix: Firestore Client caching stub in bad state issue * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * fix: Firestore Client caching stub in bad state issue * cleanup and test * Add logging --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: Mark Duckworth <[email protected]>
1 parent 3955811 commit 04ad0a4

File tree

2 files changed

+69
-6
lines changed

2 files changed

+69
-6
lines changed

dev/src/v1/firestore_client.ts

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import type {
3131
import {Transform, PassThrough} from 'stream';
3232
import * as protos from '../../protos/firestore_v1_proto_api';
3333
import jsonProtos = require('../../protos/v1.json');
34+
import {logger} from '../logger';
3435

3536
/**
3637
* Client JSON configuration object, loaded from
@@ -62,6 +63,7 @@ export class FirestoreClient {
6263
private _defaults: {[method: string]: gax.CallSettings};
6364
private _universeDomain: string;
6465
private _servicePath: string;
66+
private _stubFailed = false;
6567
auth: gax.GoogleAuth;
6668
descriptors: Descriptors = {
6769
page: {},
@@ -289,10 +291,13 @@ export class FirestoreClient {
289291
*/
290292
initialize() {
291293
// If the client stub promise is already initialized, return immediately.
292-
if (this.firestoreStub) {
294+
if (this.firestoreStub && !this._stubFailed) {
293295
return this.firestoreStub;
294296
}
295297

298+
// Reset _stubFailed because we are re-attempting create
299+
this._stubFailed = false;
300+
296301
// Put together the "service stub" for
297302
// google.firestore.v1.Firestore.
298303
this.firestoreStub = this._gaxGrpc.createStub(
@@ -328,8 +333,8 @@ export class FirestoreClient {
328333
];
329334
for (const methodName of firestoreStubMethods) {
330335
const callPromise = this.firestoreStub.then(
331-
stub =>
332-
(...args: Array<{}>) => {
336+
stub => {
337+
return (...args: Array<{}>) => {
333338
if (this._terminated) {
334339
if (methodName in this.descriptors.stream) {
335340
const stream = new PassThrough({objectMode: true});
@@ -347,9 +352,19 @@ export class FirestoreClient {
347352
}
348353
const func = stub[methodName];
349354
return func.apply(stub, args);
350-
},
351-
(err: Error | null | undefined) => () => {
352-
throw err;
355+
};
356+
},
357+
(err: Error | null | undefined) => {
358+
this._stubFailed = true;
359+
logger(
360+
'initialize',
361+
null,
362+
'Failed to create the gax client stub.',
363+
err
364+
);
365+
return () => {
366+
throw err;
367+
};
353368
}
354369
);
355370

dev/test/gapic_firestore_v1.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import * as firestoreModule from '../src/v1';
2626
import {PassThrough} from 'stream';
2727

2828
import {protobuf, LocationProtos} from 'google-gax';
29+
import {expect} from 'chai';
2930

3031
// Dynamically loaded proto JSON is needed to get the type information
3132
// to fill in default values for request objects
@@ -3173,4 +3174,51 @@ describe('v1.FirestoreClient', () => {
31733174
);
31743175
});
31753176
});
3177+
3178+
describe('initialize', () => {
3179+
it('retries on error', async () => {
3180+
const client = new firestoreModule.FirestoreClient({
3181+
credentials: {client_email: 'bogus', private_key: 'bogus'},
3182+
projectId: 'bogus',
3183+
});
3184+
const createStubStub = sinon.stub();
3185+
createStubStub.returns(Promise.reject('error'));
3186+
3187+
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
3188+
// @ts-ignore -- typing not required for testing
3189+
client['_gaxGrpc'] = {
3190+
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
3191+
// @ts-ignore -- typing not required for testing
3192+
createStub: createStubStub,
3193+
};
3194+
3195+
try {
3196+
await client.initialize();
3197+
} catch (e: unknown) {
3198+
expect(e).to.equal('error');
3199+
}
3200+
3201+
try {
3202+
await client.initialize();
3203+
} catch (e: unknown) {
3204+
expect(e).to.equal('error');
3205+
}
3206+
3207+
expect(createStubStub.callCount).to.equal(2);
3208+
});
3209+
3210+
it('does not recreate stub when one exists', async () => {
3211+
const client = new firestoreModule.FirestoreClient({
3212+
credentials: {client_email: 'bogus', private_key: 'bogus'},
3213+
projectId: 'bogus',
3214+
});
3215+
3216+
const createStubSpy = sinon.spy(client['_gaxGrpc'], 'createStub');
3217+
3218+
await client.initialize();
3219+
await client.initialize();
3220+
3221+
expect(createStubSpy.callCount).to.equal(1);
3222+
});
3223+
});
31763224
});

0 commit comments

Comments
 (0)