fix: support gRPC endpoints hosted under a URL subpath#7736
Draft
sanish-bruno wants to merge 1 commit intousebruno:mainfrom
Draft
fix: support gRPC endpoints hosted under a URL subpath#7736sanish-bruno wants to merge 1 commit intousebruno:mainfrom
sanish-bruno wants to merge 1 commit intousebruno:mainfrom
Conversation
- Enhanced the gRPC client to handle URL subpaths, allowing for proper request path construction when connecting to servers behind a subpath. - Introduced a new method to apply path prefixes to gRPC reflection clients. - Updated tests to verify the correct inclusion and case sensitivity of subpaths in gRPC request paths.
sanish-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno,
naman-bruno and
sid-bruno
as code owners
Contributor
WalkthroughUpdated gRPC URL parsing to preserve mixed-case URLs and detect protocol from parsed components. Added pathPrefix support to reflection client, enabling method path rewriting for servers hosted behind URL subpaths. Reflection now prefixes service methods with the URL subpath and performs proper channel cleanup. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as GrpcClient
participant Parser as URL Parser
participant Reflection as GrpcReflection
participant gRPC as gRPC Server
Client->>Parser: startConnection(url: "....:443/my-subpath")
Parser-->>Client: { protocol, host, path: "/my-subpath" }
Client->>Reflection: `#getReflectionClient`(url, pathPrefix: "/my-subpath")
Reflection->>Reflection: Load service definitions
Reflection->>Reflection: `#applyPathPrefix` (rewrite paths)
Note over Reflection: /my-subpath prefixed to all method paths
Reflection-->>Client: Updated reflection client
Client->>gRPC: makeUnaryRequest(path: "/my-subpath/Service/Method")
gRPC-->>Client: Response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
sanish-bruno
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bruno-requests/src/grpc/grpc-client.spec.js`:
- Around line 724-807: The test suite adds URL subpath coverage for
startConnection but misses a similar test for reflection-based method discovery,
so add a spec that calls loadMethodsFromReflection with a request URL containing
a subpath (e.g., 'grpcs://host:443/prefix'), using the same baseCollection and
mocked reflection response, then assert that capturedRequestPath includes the
subpath (e.g.,
'/prefix/grpc.reflection.v1alpha.ServerReflection/ServerReflectionInfo' or the
appropriate reflection method path), that capturedHost is 'host:443', and that
grpcClient loaded the expected methods; reference loadMethodsFromReflection,
startConnection, capturedRequestPath, capturedHost, and the mocked
grpcClient.methods.setup used in the file to locate where to add the new test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6cc43efb-6fc7-4b8e-be24-560a02b0b8cd
📒 Files selected for processing (2)
packages/bruno-requests/src/grpc/grpc-client.jspackages/bruno-requests/src/grpc/grpc-client.spec.js
Comment on lines
+724
to
+807
| describe('URL subpath support in startConnection', () => { | ||
| const baseCollection = { | ||
| uid: 'test-collection-uid', | ||
| pathname: '/test/path' | ||
| }; | ||
|
|
||
| beforeEach(() => { | ||
| grpcClient.methods.set('/test.Service/TestMethod', { | ||
| path: '/test.Service/TestMethod', | ||
| requestStream: false, | ||
| responseStream: false, | ||
| requestSerialize: (val) => Buffer.from(JSON.stringify(val)), | ||
| responseDeserialize: (val) => JSON.parse(val.toString()) | ||
| }); | ||
| }); | ||
|
|
||
| test('should include URL subpath in the gRPC request path', async () => { | ||
| const request = { | ||
| url: 'grpcs://myserver:443/my-subpath', | ||
| uid: 'test-request-uid', | ||
| method: '/test.Service/TestMethod', | ||
| headers: {}, | ||
| body: { grpc: [{ content: '{}' }] } | ||
| }; | ||
|
|
||
| await grpcClient.startConnection({ | ||
| request, | ||
| collection: baseCollection | ||
| }); | ||
|
|
||
| expect(capturedRequestPath).toBe('/my-subpath/test.Service/TestMethod'); | ||
| }); | ||
|
|
||
| test('should preserve URL subpath case sensitivity', async () => { | ||
| const request = { | ||
| url: 'grpcs://myserver:443/MySubPath', | ||
| uid: 'test-request-uid', | ||
| method: '/test.Service/TestMethod', | ||
| headers: {}, | ||
| body: { grpc: [{ content: '{}' }] } | ||
| }; | ||
|
|
||
| await grpcClient.startConnection({ | ||
| request, | ||
| collection: baseCollection | ||
| }); | ||
|
|
||
| expect(capturedRequestPath).toBe('/MySubPath/test.Service/TestMethod'); | ||
| }); | ||
|
|
||
| test('should work without subpath (standard URL)', async () => { | ||
| const request = { | ||
| url: 'grpc://myserver:50051', | ||
| uid: 'test-request-uid', | ||
| method: '/test.Service/TestMethod', | ||
| headers: {}, | ||
| body: { grpc: [{ content: '{}' }] } | ||
| }; | ||
|
|
||
| await grpcClient.startConnection({ | ||
| request, | ||
| collection: baseCollection | ||
| }); | ||
|
|
||
| expect(capturedRequestPath).toBe('/test.Service/TestMethod'); | ||
| }); | ||
|
|
||
| test('should connect to host without subpath in channel target', async () => { | ||
| const request = { | ||
| url: 'grpcs://myserver:443/my-subpath', | ||
| uid: 'test-request-uid', | ||
| method: '/test.Service/TestMethod', | ||
| headers: {}, | ||
| body: { grpc: [{ content: '{}' }] } | ||
| }; | ||
|
|
||
| await grpcClient.startConnection({ | ||
| request, | ||
| collection: baseCollection | ||
| }); | ||
|
|
||
| expect(capturedHost).toBe('myserver:443'); | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
Add a reflection-specific subpath test as well.
These cases cover startConnection, but the new behavior in this PR also changes loadMethodsFromReflection() for subpath-hosted servers. Without one test around reflection on a URL like grpcs://host/prefix, method discovery can regress while this suite still passes.
As per coding guidelines, **/*.{test,spec}.{js,jsx,ts,tsx}: Add tests for any new functionality or meaningful changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bruno-requests/src/grpc/grpc-client.spec.js` around lines 724 - 807,
The test suite adds URL subpath coverage for startConnection but misses a
similar test for reflection-based method discovery, so add a spec that calls
loadMethodsFromReflection with a request URL containing a subpath (e.g.,
'grpcs://host:443/prefix'), using the same baseCollection and mocked reflection
response, then assert that capturedRequestPath includes the subpath (e.g.,
'/prefix/grpc.reflection.v1alpha.ServerReflection/ServerReflectionInfo' or the
appropriate reflection method path), that capturedHost is 'host:443', and that
grpcClient loaded the expected methods; reference loadMethodsFromReflection,
startConnection, capturedRequestPath, capturedHost, and the mocked
grpcClient.methods.setup used in the file to locate where to add the new test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #7699
Summary
grpc://host/api/grpc)url.toLowerCase()incorrectly lowercasing case-sensitive URL subpathsProblem
When a gRPC service is exposed behind a URL subpath (e.g.,
grpc://localhost:4001/api/grpc), Bruno failed withstatus code 12 (UNIMPLEMENTED). The root cause was that the reflection client did not include the subpath prefix in its method paths — so loading methods via reflection failed before any request could be made.Additionally,
getParsedGrpcUrlObject()calledurl.toLowerCase()on the entire URL, which could break case-sensitive subpath routing.Changes
packages/bruno-requests/src/grpc/grpc-client.js:#applyPathPrefixmethod: Replaces theGrpcReflectionlibrary's internal gRPC client with one whose method paths include the URL subpath prefix. This enables reflection to work behind subpath proxies.#getReflectionClient: Accepts and forwards apathPrefixparameter.loadMethodsFromReflection: Passes the parsed URL path to#getReflectionClient.getParsedGrpcUrlObject: Removedurl.toLowerCase()to preserve case-sensitive subpaths. Protocol is normalized to lowercase separately.generateGrpcurlCommand: Protocol detection now uses the parsedprotocolfield instead ofurl.startsWith().packages/bruno-requests/src/grpc/grpc-client.spec.js:How it works
For a URL like
grpc://localhost:4001/api/grpc:localhost:4001(host only, no subpath)/api/grpc/grpc.reflection.v1.ServerReflection/ServerReflectionInfo/api/grpc/package.Service/MethodContribution Checklist: