deferred client initialization

[alexander-fenster]

We need to move all asynchronous code (promises creation) from the client constructor to the separate init method that will be called by all class methods (once, at the first request). Cloud functions don't like asynchronous stuff in constructors.

To be discussed:

• if this deferred initialization should be default, optional, or be enabled only within the cloud functions
• if we want to make all our client methods async (possibly dropping callbacks)

[bcoe]

We think this might be a potential solution to googleapis/google-auth-library-nodejs#798

The root problem being that new SomeClient() creates asynchronous work which results in background work being scheduled and throttled.

A work around people can test in the meantime is the following:

const {Storage} = require('@google-cloud/storage');
const {VisionClient} = require('@google-cloud/vision');
let storage;
let vision;
exports.handler = async (req, res) {
  if (!storage) {
    storage = new Storage();
    vision = new Vision();
  }
  await storage.someOperation();
  await vision.someOperation();
}

Lazy initialize any clients you are creating, such that the new step occurs inside your handler, rather than in the global scope of your script.

@adamworrall, @kmilo93sd, @SaschaHeyer, @bdaz, @elihorne, I wanted to put this potential fix on your radar. Also, could I bother you to try the suggestion I link to in this comment? If it makes you're issues go away, it's a good indicator that we're on the right track.

[adamworrall]

@bcoe I haven't seen the error pop up for a while but I also made this change for most of my functions just to see if it made a difference. I suspect you're right about what's going on here. The first time I noticed the issue was after adding the google-logging/winston package, and initializing the client in the global scope.

It should be noted that initializing the client in global scope is specifically recommended in the GCF docs:
https://cloud.google.com/functions/docs/bestpractices/networking#accessing_google_apis

Creating a PubSub client object results in one connection and two DNS queries per invocation. To avoid unnecessary connections and DNS queries, create the PubSub client object in global scope as shown in the sample below:

I wonder if something has changed since those docs were written, maybe in the way Node 10 functions are executed? Docs should probably be updated, though.

Thanks very much for your diligence, this error has been a source of anxiety for quite a while. I'll followup if it returns for a function with no async work in the global scope.

[bcoe]

@adamworrall we haven't yet been able to update some of the libraries, as we're holding out to drop Node 8 support, before releasing any that need a major bump.

However, for a healthy number of our client libraries we've released Alex's deferred client initialization fix. This should mean that, in the near future, you would be able to go back to initializing a client in the global scope, and as long as the first operation is performed int he handler function, things will behave as expected.

[dknedlik]

Still getting this Could not load the default credentials error and I can't figure out a way around it. 100% of the time touching firestore fails after a cold start.

Every function I have that uses firestore crashes on first run, they randomly then crash on subsequent runs. I have even put together a simple test to just write and trigger from firestore, removing any business logic or custom code.

const Firestore = require('@google-cloud/firestore');
let fs;

exports.qboCallBack = functions.https.onRequest(async (req,res) => {
 if(!fs){
    fs = new Firestore();
  }
 //create the worker job to collect the company data`
 const docref =  await fs.collection("getCompanyDataJobs").add({
      realmId: 1234,
      complete: false,
      token: 1234
    })
    res.redirect(
      `${BASE_URL}/company?realmid=${'1234'}&jobtype=getCompanyDataJobs&jobid=${docref.id}`
    );
})

exports.loadCompanyDataTrigger = functions.firestore.document('getCompanyDataJobs/{jobId}').onCreate(async (snap, context) => { 
  if(!fs){
    new Firestore();
  }
  await fs.collection("getCompanyDataJobs").add({
    realmId: 1234,
    complete: true,
    token: 1234
  })
});

[alexander-fenster]

@dknedlik This might not be the best place to discuss Firestore since it has a lot of code wrapping the generated library. Let's summon @schmidt-sebastian here and he might decide to move this discussion to the Firestore repository :)

[dknedlik]

@alexander-fenster Thanks appreciate it. I have been chasing this for quite awhile and I ended up here from another issue that was also discussing firestore but the issue has been locked.
googleapis/google-auth-library-nodejs#798

[schmidt-sebastian]

@dknedlik I might be able to look at this over in https://github.com/googleapis/nodejs-firestore. Please include log messages (obtained via setLogFunction) if you do decide to file an issue.

[dknedlik]

@schmidt-sebastian thank you. I was able to get it working. Turned out to be a globally constructed cloud Logger instance in another file that was causing the issues. I was able to move construction of the cloud logger into a function scope and that finally cleared up the last of the issues.

[alexander-fenster]

@dknedlik That explains a lot. Logger (nodejs-logging) also received the deferred initialization fix, but it was not released yet (going to be released within a week or two). It might just solve the problems then.

[markterrill]

Hi, this and the other thread have been useful! I've moved @google-cloud/logging to lazy init.

However my issue is with the no default creds error.

What is the best practice for this standard pattern:

// Create a new Cloud IoT client
const auth = await google.auth.getClient({
	scopes: ['https://www.googleapis.com/auth/cloud-platform']
});
const googleClient = google.cloudiot({
	version: 'v1',
	auth: auth
});

Appreciate any guidance!

[alexander-fenster]

@markterrill From your snippet it looks like you're using googleapis package for Cloud IOT. Is it a requirement? We have a separate library published for IOT: https://www.npmjs.com/package/@google-cloud/iot which uses the same authentication approach as @google-cloud/logging.

[markterrill]

Let me investigate! Tha

@markterrill From your snippet it looks like you're using googleapis package for Cloud IOT. Is it a requirement? We have a separate library published for IOT: https://www.npmjs.com/package/@google-cloud/iot which uses the same authentication approach as @google-cloud/logging.

Let me investigate, thanks!

I've been using it for:

client.projects.locations.registries.devices.modifyCloudToDeviceConfig(request, (err, resp) => {

ok, found this: https://googleapis.dev/nodejs/iot/latest/google.cloud.iot.v1.DeviceManager.html#modifyCloudToDeviceConfig1

[markterrill]

Cheers. Updated, working, pushed to beta.