Skip to content

Is the example rpcImpl in the README.md correct? #1229

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.

Sign up for GitHub

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

Jump to bottom
Open
GeorgeBills opened this issue May 26, 2019 · 8 comments · May be fixed by #2046
Open

Is the example rpcImpl in the README.md correct? #1229

GeorgeBills opened this issue May 26, 2019 · 8 comments · May be fixed by #2046

Comments

@GeorgeBills
Copy link

The example given is:

const rpcImpl = function(method, requestData, callback) {
  client.makeUnaryRequest(
    method.name,
    arg => arg,
    arg => arg,
    requestData,
    callback
  )
}

That gives a method call to the path "SayHello" which won't actually work against the helloworld example server - it needs to include the namespace and service in the path: "/helloworld.Greeter/SayHello".

The below seems to work for me:

const rpcImpl = function (method, requestData, callback) {
    const service = method.parent, namespace = service.parent;
    client.makeUnaryRequest(
        "/" + namespace.name + "." + service.name + "/" + method.name,
        arg => arg,
        arg => arg,
        requestData,
        callback
    )
}
@longility
Copy link

I spent about a day's worth of work before figuring this out on my own. I searched here to find that I'm not the only one. Is this the correct way? If so, we should update the example.

@longility
Copy link

method.parent do not exist anymore. I had to hardcode for now.

@Mohammad-Khalid23
Copy link

@longility
`package parentName;

syntax = "proto3";`

if you will use this in your proto file then you will get method.parent

@longility
Copy link

I do have that @Mohammad-Khalid23 . I'm generating my code like this, which works, but maybe I'm missing something:

pbjs -t static-module -w commonjs -o src/junk-domain/__grpc-stubs__/profile.js ../profile/*.Api/Protos/*.proto
pbts -o src/junk-domain/__grpc-stubs__/profile.d.ts src/junk-domain/__grpc-stubs__/profile.js

I followed this: https://www.npmjs.com/package/protobufjs#pbts-for-typescript

@morcam
Copy link

morcam commented Dec 5, 2020

Yeah, I ran into this today as well. I don't have any parent value either. Assuming that there are indeed some cases where that parent value isn't present, isn't this an actual problem with the library, rather than just with the example? Or is the intended usage pattern to capture the namespace & service class in the RPC implementation? That would seem a bit odd to me, given that the name is passed in already.

@Raylin51
Copy link

Raylin51 commented Mar 2, 2021

I have the same problem. Is it resolved?

@hermanbanken
Copy link

Correct, this example is no longer accurate. Even the gRPC library is updated and now has more fields on makeUnaryRequest.

@adam-rocska
Copy link

adam-rocska commented May 27, 2024 •
edited
Loading

For f’s sake, since 2021 nobody updated the docs nor solved this issue. Do we really have to manually hack our paths in the fng call, or there is another ticket hidden from the internet that explains how it’s supposed to be used in 2024?

@hansp27 hansp27 linked a pull request Mar 3, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(docs): fix invalid method path and update grpc npm package hansp27/protobuf.js
7 participants
@longility @hermanbanken @GeorgeBills @adam-rocska @Raylin51 @Mohammad-Khalid23 @morcam