feat: Streamline gRPC requests to use right context#6308
feat: Streamline gRPC requests to use right context#6308bijin-bruno merged 1 commit intousebruno:mainfrom
Conversation
WalkthroughAdds collection-, folder-, and request-level variable sources into string interpolation and wires them into certificate/proxy interpolation; gRPC IPC handlers normalize missing URL protocol and pass Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (1)**/*.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
🔇 Additional comments (3)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
packages/bruno-electron/src/ipc/network/grpc-event-handlers.js (1)
184-197: Same double merge issue as in grpc:start-connection.This handler repeats the problematic pattern from lines 40-52, resulting in duplicate variable merging and code duplication.
Apply the same fix as suggested for the previous handler.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/bruno-electron/src/ipc/network/cert-utils.js(1 hunks)packages/bruno-electron/src/ipc/network/grpc-event-handlers.js(4 hunks)packages/bruno-electron/src/ipc/network/index.js(1 hunks)packages/bruno-electron/src/ipc/network/interpolate-string.js(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation, never tabs
Use single quotes for strings instead of double quotes
Always add semicolons at the end of statements
No trailing commas in code
Always use parentheses around parameters in arrow functions, even for single parameters
For multiline constructs, put opening braces on the same line with a minimum of 2 elements for multiline formatting
No newlines inside function parentheses
Space before and after the arrow in arrow functions:() => {}
No space between function name and parentheses:func()notfunc ()
Semicolons should go at the end of the line, not on a new line
Function names should be concise and descriptive
Add JSDoc comments to provide details to abstractions
Avoid single-line abstractions where all that is being done is increasing the call stack with one additional function
Add meaningful comments to explain complex code flow instead of obvious comments
Files:
packages/bruno-electron/src/ipc/network/cert-utils.jspackages/bruno-electron/src/ipc/network/grpc-event-handlers.jspackages/bruno-electron/src/ipc/network/index.jspackages/bruno-electron/src/ipc/network/interpolate-string.js
🧬 Code graph analysis (2)
packages/bruno-electron/src/ipc/network/cert-utils.js (3)
packages/bruno-electron/src/ipc/network/index.js (4)
collectionVariables(303-303)collection(146-146)requestVariables(302-302)request(326-326)packages/bruno-cli/src/utils/collection.js (3)
collectionVariables(159-159)folderVariables(166-166)requestVariables(167-167)packages/bruno-electron/src/ipc/network/interpolate-vars.js (3)
collectionVariables(24-24)folderVariables(25-25)requestVariables(26-26)
packages/bruno-electron/src/ipc/network/grpc-event-handlers.js (3)
packages/bruno-electron/src/ipc/network/prepare-grpc-request.js (3)
requestTreePath(24-24)collection(21-21)prepareGrpcRequest(16-122)packages/bruno-electron/src/utils/collection.js (2)
getTreePathFromCollectionToItem(239-247)mergeVars(52-98)packages/bruno-cli/src/utils/collection.js (2)
getTreePathFromCollectionToItem(333-341)mergeVars(155-201)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - Linux
- GitHub Check: Playwright E2E Tests
- GitHub Check: CLI Tests
- GitHub Check: Unit Tests
🔇 Additional comments (4)
packages/bruno-electron/src/ipc/network/cert-utils.js (1)
50-59: LGTM on interpolationOptions structure.Once the variable initialization is corrected, this properly wires the new variable sources into the interpolation context for certificate and proxy configuration.
packages/bruno-electron/src/ipc/network/index.js (1)
112-114: LGTM on URL normalization guard.The
hasVariablescheck prevents protocol normalization for templated URLs, preserving them for later interpolation. The implementation is straightforward and aligns with the PR objectives.packages/bruno-electron/src/ipc/network/interpolate-string.js (2)
4-14: LGTM on expanded function signature.The destructured parameters cleanly expose all variable sources for interpolation.
19-56: LGTM on variable initialization and merging.The new variable sources are properly initialized with defaults and merged into
combinedVarswith the correct precedence order. The implementation is consistent with the existing pattern.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/bruno-electron/src/ipc/network/grpc-event-handlers.js (1)
39-42: Avoid duplicating protocol normalization logic in three handlersThe same
protocolRegexandhttp://prefixing logic is repeated in all three handlers (start-connection,load-methods-reflection, andgenerate-grpcurl). Centralising this (either as a shared top‑level helper/constant in this module or insideprepareGrpcRequest) would keep URL rules consistent and reduce future maintenance overhead if the normalization rules evolve again.Also applies to: 176-180, 287-290
🧹 Nitpick comments (1)
packages/bruno-electron/src/ipc/network/grpc-event-handlers.js (1)
287-290: Extend grpcurl interpolation options to use new variable sourcesIn
grpc:generate-grpcurl,interpolationOptionsonly includesenvVars,runtimeVariables, andprocessEnvVars, so the new variable categories supported byinterpolateString(e.g. collection‑, folder‑, request‑, and global environment variables / prompt variables) are not used when resolvingclientCert.domain,certFilePath, andkeyFilePath. That means grpcurl’s certificate path interpolation is now less capable than the main cert/proxy flow viagetCertsAndProxyConfig.Consider expanding
interpolationOptionshere to pass through the same variable sets thatgetCertsAndProxyConfigis using, so domain and path templates behave consistently across request execution and grpcurl generation. While you’re here, double‑check that the host‑matching regex in this block stays in sync with the scheme you now normalize to (http://when missing), or relax the scheme matching so that normalized URLs still match configured domains.Also applies to: 292-296, 303-320
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/bruno-electron/src/ipc/network/grpc-event-handlers.js(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation, never tabs
Use single quotes for strings instead of double quotes
Always add semicolons at the end of statements
No trailing commas in code
Always use parentheses around parameters in arrow functions, even for single parameters
For multiline constructs, put opening braces on the same line with a minimum of 2 elements for multiline formatting
No newlines inside function parentheses
Space before and after the arrow in arrow functions:() => {}
No space between function name and parentheses:func()notfunc ()
Semicolons should go at the end of the line, not on a new line
Function names should be concise and descriptive
Add JSDoc comments to provide details to abstractions
Avoid single-line abstractions where all that is being done is increasing the call stack with one additional function
Add meaningful comments to explain complex code flow instead of obvious comments
Files:
packages/bruno-electron/src/ipc/network/grpc-event-handlers.js
🧬 Code graph analysis (1)
packages/bruno-electron/src/ipc/network/grpc-event-handlers.js (3)
packages/bruno-electron/src/ipc/network/index.js (4)
protocolRegex(111-111)certsAndProxyConfig(117-126)certsAndProxyConfig(147-147)collection(146-146)packages/bruno-electron/src/ipc/network/cert-utils.js (2)
getCertsAndProxyConfig(12-126)collection(44-44)packages/bruno-electron/src/ipc/network/prepare-grpc-request.js (1)
collection(21-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: CLI Tests
- GitHub Check: Playwright E2E Tests
- GitHub Check: Unit Tests
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - Windows
🔇 Additional comments (1)
packages/bruno-electron/src/ipc/network/grpc-event-handlers.js (1)
45-53: PassingpreparedRequestintogetCertsAndProxyConfiglooks correctRouting
preparedRequest(rather than the raw request) intogetCertsAndProxyConfigensures certificate/proxy configuration is based on the fully prepared URL and variable-expanded fields, which matches how HTTP request configuration is wired elsewhere and should make cert selection more reliable.Also applies to: 182-191
72e5cf0 to
b87a02b
Compare
Jira
fixes: #6254
Description
Streamline gRPC requests to use right context
Contribution Checklist:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.