grpc@1.8.0 -- cannot read property 'name' of null

[ntindall]

We are seeing this error with grpc@1.8.0. It looks like arguejs may have some difficulty resolving the argument order inside makeUnaryRequest.

"TypeError: Cannot read property 'name' of null
    at (/srv/www/link/releases/20171219165514/node_modules/plaid-escalate client/node_modules/grpc/node_modules/arguejs/argue.js:117:46)
    at ServiceClient.Client.makeUnaryRequest (/srv/www/link/releases/20171219165514/node_modules/plaid-escalate-client/node_modules/grpc/src/client.js:507:14)
    at apply (/srv/www/link/releases/20171219165514/node_modules/plaid-escalate-client/node_modules/grpc/node_modules/lodash/lodash.js:499:17)
    at ServiceClient.wrapper (/srv/www/link/releases/20171219165514/node_modules/plaid-escalate-client/node_modules/grpc/node_modules/lodash/lodash.js:5356:16)

It seems like this is a known issue with arguejs: zvictor/ArgueJS#23.
The "bug" is just that it does an invalid dereference before it can throw a more useful error. If you change the problem line there to print out "foobar" instead of throwing a NPE, we end up with something like this:

"Error: Incompatible type signature. Expecting ( String, Function, Function, foobar, [Metadata], [Object], Function ), given ( String, Function, Function, Object, Object, Object, Function )

It seems like this is still an "irrecoverable error" type scenario, though.

here is how we are creating the client (typescript)

const root = new $protobufjs.Root();
root.resolvePath = (origin, target) =>
  path.resolve(PATH_TO_PROTO) + '/' + target;
root.loadSync(GRPC_SERVICE_DEFINITION', {
  keepCase: true,
});

const proto = grpc.loadObject(root, {
  enumsAsStrings: false,
  protobufjsVersion: 6,
});

We implement a custom promisification wrapper around the grpc prototype... at the end of the day it looks something like:

function grpcUnaryPromisifier(func: grpc.Request): any {
  // tslint:disable-next-line only-arrow-functions
  return async function promisified(
    // The argument to use
    argument: any,

    // The grpc metadata object, if undefined -- grpc will default to creation
    // of new metadata.
    metadata?: grpc.Metadata,

    // Options for this rpc
    options: grpc.CallOptions = ({} as grpc.CallOptions),
  ): P<any> {
    if (metadata == null) {
      metadata = new grpc.Metadata();
    }

    // tslint:disable no-invalid-this
    const self: EscalateClientInternal = this;
    // tslint:enable no-invalid-this

    // Populate default values into the options.
    options = R.merge({
      deadline: Date.now() + DEFAULT_TIMEOUT_MS,
    }, options);

    // do some logging, retry with exponential backoff, etc.
    try {
      const result = await P.fromCallback(cb => func.call(
            self, argument, metadata, options, cb));
    } catch (e) {
      // log and retthrow
    }

   // return the result
}

This all works fine and dandy with the client using 1.7.3...

     const metadata = new grpc.Metadata();

     if (typeof context.requestId === 'string') {
       metadata.set('request_id', context.requestId);
     }

     plaidGrpcService.client()
     .nameOfPromisifiedMethodAsync(requestObject, metadata, {
       deadline: plaidGrpcService.defaultDeadline(),
     })
     .then(res => {
       // business logic.
     })

I initially thought that there was some problem because the calling service was using grpc@1.8.0 despite the client using grpc@1.7.3. Since npm does not flatten dependencies, this should be a valid use case.

The problem first arose when we were passing a grpc.Metadata that was created by 1.8.0 to this client implementation using 1.7.3. To test whether this is the problem, I tried changing both the calling service and the client to use grpc@1.8.0 and this issue persists. With both the service and the client using 1.7.3, there is no issue. I have not tested what happens when the client uses 1.8.0 and the service uses 1.7.3.

Based upon the error I was able to coerce out of arguejs, it looks as if it is not recognizing that the Object passed in is actually a Metadata. Why this would be, I am not sure. Note that the version of arguejs that grpc uses is constant between 1.7.3 and 1.8.0.

I looked at the diff between v1.7.3 and v1.8.0 and it seems fairly innocuous, so I am rather at a loss here.

Help would be appreciated.

[murgatroid99]

I'm not entirely clear on what you're doing. When you say "we were passing a grpc.Metadata that was created by 1.8.0 to this client implementation using 1.7.3", do you mean that you are taking a Metadata object created from grpc 1.8.0, and passing it to a grpc 1.7.3 function? If so, that would cause the problem you are seeing. I am not really sure why you would be seeing the same error if you are using the same grpc library for both. Is it possible that you are loading two separate instances of grpc 1.8.0?

[ntindall]

do you mean that you are taking a Metadata object created from grpc 1.8.0, and passing it to a grpc 1.7.3 function?

yes. that is what we were doing... it makes sense to me that this could cause problems, but I'm not sure this is the ideal world. It would be best if we had backwards compatibility here, right? to be clear:

     const metadata = new grpc.Metadata(); // grpc 1.8.0 metadata

     if (typeof context.requestId === 'string') {
       metadata.set('request_id', context.requestId);
     }
     
     // promisified grpc 1.7.3 function
     plaidGrpcService.client()
     .nameOfPromisifiedMethodAsync(requestObject, metadata, {
       deadline: plaidGrpcService.defaultDeadline(),
     })
     .then(res => {
       // business logic.
     })

Is it possible that you are loading two separate instances of grpc 1.8.0?

I think that could be possible. Let me try to reproduce it again with everything using 1.8.0.

In general @murgatroid99, is this a supported use case or should we be aiming to have everything using the same grpc version?

---

Edit: OK, I tried it again, this time ensuring there was no duplicate instance of grpc, and this is no longer a problem. Do you have any ideas about why this fixes it?

[murgatroid99]

For the foreseeable future, you should not expect to be able to mix together objects from different instances of the gRPC library. It would make type checking a nightmare, and some types are implemented with underlying C structures and therefore simply would not be portable at all.

[ntindall]

Ok, thanks for your help @murgatroid99. I am wondering if there is anything that could be done in order to make the root cause here more obvious to others who maybe encountering this... Assuming the bug in arguejs were fixed, do you think this is a good failure mode?