Google-cloud-node: Latest pubsub version breaks dependencies with grpc (doesn't use common-grpc package)

[lostpebble]

Environment details

• OS: Linux
• Node.js version: 8.7.0
• npm version: 5.4.2
• google-cloud-node version:

Steps to reproduce

Try use the latest versions of @google-cloud/datastore and @google-cloud/pubsub together.

Because the latest version of @google-cloud/pubsub includes grpc as a direct dependency instead of using the package which seems to have been created for linking grpc together between libraries (@google-cloud/common-grpc), the versions of GRPC are clashing and causing problems such as #2667

To get my project working I had to revert to @google-cloud/pubsub:0.14.2

It appears to have been changed during this commit: #2627

[ThomWright]

I can reproduce with the following in my package.json:

"@google-cloud/pubsub": "^0.14.5",
"grpc": "^1.6.6",

Fixing @google-cloud/pubsub:0.14.2 worked for me too.

[watermanio]

I think I'm seeing something related using PubSub on GCF with @google-cloud/pubsub:0.14.6:

OS: OS X
Node.js version: v6.11.1 (for GCF)

var Buffer = require("safe-buffer").Buffer;
var pubsub = require('@google-cloud/pubsub')();

exports.publish = function publish(event, callback) {
    var topic = pubsub.topic('outbound-topic');
    var publisher = topic.publisher();
    var message = new Buffer('New message!');
    publisher.publish(message, function(err, data) {
        callback(err, data);
    });
};

Throws the following error:

info: Function crashed
info: TypeError: message.encode(...).finish is not a function
    at BundleDescriptor.getByteLength [as byteLengthFunction] (./node_modules/google-gax/lib/grpc.js:262:36)
    at ./node_modules/google-gax/lib/bundling.js:338:38
    at Array.forEach (native)
    at BundleExecutor.schedule (./node_modules/google-gax/lib/bundling.js:337:16)
    at ./node_modules/google-gax/lib/bundling.js:474:20
    at Canceller.call (./node_modules/google-gax/lib/api_callable.js:110:19)
    at Bundleable.call (./node_modules/google-gax/lib/bundling.js:473:12)
    at ./node_modules/google-gax/lib/api_callable.js:356:17

[stephenplusplus]

cc @jsididris who reported this in #2717.

[callmehiphop]

@lostpebble I just cut a new patch release. If upgrading to that package does not fix your issue, feel free to re-open.

[stephenplusplus]

@lostpebble RE: #2738, I take it this didn't help anything?

rm -rf node_modules
npm cache clean
npm install

[lostpebble]

@stephenplusplus I copied my project over to a fresh space and reinstalled all modules and it seems to be working fine now. Thanks!

Although, my actual dev environment is a yarn workspace - which seems to be getting confused about where to place the dependencies. I'll have to look into it a bit more.

[lostpebble]

@stephenplusplus So, through a lot of trial and error, trying every yarn command known to man and failing, and eventually having to just modify the yarn.lock file directory directly, I eventually got my project to see grpc:1.7.1 as a root dependency and PubSub loads up without that error now.

But I am seeing a new strange error popping up:

TypeError: grpc.forcePoll is not a function

Which I've tracked to this line of code in grpc-node : https://github.com/grpc/grpc-node/blob/2b538ca0217638b3f0f45158b53eb9cd912381fb/packages/grpc-native-core/src/client.js#L805

This happens after setting up and adding listeners to PubSub.

The listeners are working however and everything seems fine otherwise... So going to roll with it for now. Could be the client not initialising properly the first time, but seems to start up anyway after that.

[stephenplusplus]

That doesn't sound good.

@callmehiphop any ideas?

[callmehiphop]

@stephenplusplus that definitely looks like an internal grpc error, maybe we should pin grpc to 1.6?

[callmehiphop]

Although, maybe a better move would be to rewrite the IAM sections using GAX and cut the dependency on common-grpc altogether.

@lukesneeringer do we have timing for when PubSub will become its own repo?

[stephenplusplus]

To avoid problems for now, pinning would be a welcome, quick fix. Here are the ways grpc enters a project through the Datastore & Pub/Sub modules:

• google-gax: grpc@^1.2
• @google-cloud/common-grpc: grpc@^1.6.0

Do we need to pin to ^1.7.1 (the latest at this time) in both projects?

[callmehiphop]

@stephenplusplus I've decided to just go ahead an convert IAM to gax, pinning a version in both modules is a very fragile solution that has the potential to resurface in the very near future.

[stephenplusplus]

Why is it fragile?

The problem seems to be when a project has multiple versions of gRPC required through our dependencies. While removing common-grpc from Pub/Sub gets rid of the extra gRPC for that module, we still have it in Bigtable, for example. So if a project uses Bigtable and Datastore, they could hit these weird version mismatches.

[callmehiphop]

It's fragile in the sense that if we have to pin versions on 2 different modules and anytime one gets updated the other will need a similar update, otherwise we run the risk of seeing this issue again.

If removing common-grpc is a more reliable solution, I think its worth the effort (which in this case is minimal).

[stephenplusplus]

I think removing common-grpc is a better solution. I'm only concerned about the effort causing a long wait time for the people whose apps aren't working properly. Unless you think I'm over-estimating the amount of affected users / the effort?

I went digging for common-grpc usage:

• Bigtable
  
  
  
  • Operation()
  
  
  • Service.decorateStatus_()
  
  
  • Bigtable extends Service
  
  
  • Cluter extends ServiceObject
  
  
  • Family extends ServiceObject
  
  
  • Instance extends ServiceObject
  
  
  • Row extends ServiceObject
  
  
  • Table extends ServiceObject
  
  
• Datastore -- will not need it after repo split is complete
• Firestore
  
  
  
  • Firestore extends Service
  
  
• Logging
  
  
  
  • Service.structToObj_()
  
  
  • Service.objToStruct_()
  
  
• Spanner
  
  
  
  • Operation()
  
  
  • Service.decodeValue_()
  
  
  • Service.encodeValue_()
  
  
  • Spanner extends Service
  
  
  • Database extends ServiceObject
  
  
  • Instance extends ServiceObject
  
  
  • Session extends ServiceObject
  
  

[lostpebble]

Have you guys considered perhaps just making grpc / common-grpc a peerDependency in all those modules? Leave it up to the user to keep a single version of grpc up to date (should get warnings and be pointed out by yarn or npm on which versions are required).

I've used that tactic in my datastore library to prevent multiple versions of datastore being included and causing issues. It's the case in a lot of libraries actually, when you have multiple modules requiring some dependency, things usually get messy and peerDependencies are sometimes the best way to mitigate it.

[callmehiphop]

Ah, got it. Hard to say, we haven't really gotten any issue reports about this being a problem outside of PubSub, so I just assumed they were ok. Maybe fix PubSub for now and if this resurfaces elsewhere we can talk about trying to pin the versions?

[stephenplusplus]

That sounds like a good plan. It also wouldn't hurt to update the google-gax and common-grpc dependency ranges to ^1.7.1, so that npm doesn't pull an older version from cache to satisfy either. Having those in sync should be enough to fix this issue.

Also, if anyone is listening in here, let us know if you've had any problems using two @google-cloud modules at the same time.

@lostpebble fortunately, we're heading towards getting rid of the common-grpc module, which will save us from the peerDependencies headaches. 😌

[stephenplusplus]

To state simply what the problem is: it's possible to load multiple versions of gRPC. And these versions do not always play nicely together.

That leaves two logical solutions:

1) Make sure the versions you install are the same version (good)
2) Only install it once (best)

Someday, we will accomplish #­2. @google-cloud/common-grpc will disappear. Until then, we have to do #­1.

google-gax will update with the newest gRPC soon (https://github.com/GoogleCloudPlatform/google-cloud-node/issues/2699), and I'm about to release @google-cloud/common-grpc that depends on the newest gRPC as well.

[stephenplusplus]

Oh, and one more DIY solution, should this problem arise in the future:

$ rm -rf node_modules
$ npm cache clean
$ npm install

This should force npm to install the same gRPC versions, and not pull anything from the cache.

[ctavan]

yarn users need to make sure to remove the yarn.lock file before re-installing since otherwise yarn.lock might lock grpc down to an old version. Same might apply to package-lock.json which was introduced by npm5?!

@stephenplusplus I believe that @google-cloud/pubsub will have to be bumped once again to use a new google-gax version as soon as https://github.com/googleapis/gax-nodejs/pull/158 has been merged?

[Amit-A]

I'm still facing this issue on cloud functions.

TypeError: pubsub.topic(...).publisher is not a function
    at /srv/lib/outputs/pubsub.js:16:29

Downgrading to "@google-cloud/pubsub": "^0.21.1" fixed it for me.

[callmehiphop]

@Amit-A our latest release contains a couple of breaking changes, one of which was the removal of the publisher() method. Now to publish messages you just need to call publish()

pubsub
  .topic('my-topic')
  .publish(message, (err, messageId) => {
    // ...
  });

I apologize for any inconveniences this may have caused.

[Amit-A]

@callmehiphop ah that makes sense, thanks for letting me know!