Skip to content

grpc-reflection doesn't load properly when an imported type doesn't have a package #2934

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.

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

Open
matrosovs opened this issue Apr 1, 2025 · 1 comment

Comments

@matrosovs
Copy link

matrosovs commented Apr 1, 2025

Problem description

When imported type doesn't have a package, and it is used as a field, when I try to enable reflection, first I get these warnings:

Could not find file associated with reference NoPackage

But the gRPC service starts succesfully. However, when trying to load the service definition in Postman I get this error:

Image

Reproduction steps

I modified your reflection example to simulate the problem we have in our code. Please check here. See these 2 files:

  • examples/protos/helloworld.proto
  • examples/protos/nopackage.proto

This is what I get when I run it:

$ node ./reflection/server.js 
Debugger listening on ws://127.0.0.1:53801/5014bd58-6641-4daa-a11b-f7b03f82abcb
For help, see: https://nodejs.org/en/docs/inspector
Debugger attached.
Could not find file associated with reference NoPackage  <--- these are the warnings in question
Could not find file associated with reference NoPackage
(node:141961) DeprecationWarning: Calling start() is no longer necessary. It can be safely omitted.
(Use `node --trace-deprecation ...` to show where the warning was created)

And if I try to load the definition via reflection in postman, I will get the error on the screenshot above.

Environment

  • OS name, version and architecture: Debian GNU/Linux 12 (bookworm), AMD x64
  • Node version: v20.18.0
  • Node installation method: for this particular example I used npm, but in our project we use yarn
  • Package name and version: @grpc/[email protected]

Additional context

This seem related to this issue: #2671, but sort of in reverse. Instead of removing the dot, I need to add one.

The way I resolved it is by simply checking if there is a reference with a leading dot. See packages/grpc-reflection/src/implementations/reflection-v1.ts in the same branch:

        // if we didn't find anything then try just a FQN lookup
        if (!referencedFile) {
          referencedFile = this.symbols[ref] ?? this.symbols[`.${ref}`];
        }

I would have opened a PR with my change, but I am not sure how to test it. I tried to add a test to packages/grpc-reflection/test/test-reflection-v1-implementation.ts (and updated *.proto files to have the same setup), but regardless of whether this fix is present or not, I get the same content for reflectionService.

But if you rerun the example service with the fix, definition loads via reflection with no issues:
Image

And Postman can even generate some sample request:
Image

@matrosovs
Copy link
Author

@murgatroid99, I opened a PR for this. Feel free to review it: #2941.

I will do the EasyCLA thing tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants