fix(migration): keep the call wrapping a response member in pm.sendRequest callbacks - #9162
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. WalkthroughThe response-access transformer now uses shared, scope-aware rewriting for promise handlers and ChangesResponse rewriting
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant PostmanScript
participant sendRequestTransformer
participant rewriteResponseAccess
participant BrunoScript
PostmanScript->>sendRequestTransformer: provide handler or callback AST path
sendRequestTransformer->>rewriteResponseAccess: pass path and response parameter name
rewriteResponseAccess->>sendRequestTransformer: rewrite scoped response members
sendRequestTransformer->>BrunoScript: emit translated response access
Merge Risk: ⚪ Minimal · up to No merge-blocking behavior risk remains from the reviewed change. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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. Response paths now flow with care Comment |
37a240d to
5a1fa7d
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
…d improve response handling
5a1fa7d to
5161eaf
Compare
BRU-4291
Description
When migrating a Postman script to Bruno, the
pm.sendRequestcallback rewriter maps Postman response access to its Bruno equivalent (response.code→response.status,response.json()→response.data, ...). If a mapped response member sat inside a call as an argument, the rewriter replaced the entire wrapping call instead of just the member.Example:
Problem
The rewrite was written with
response.json()in mind. On Postman that is a method, on Bruno it is a plain property, so the call has to lose its parentheses. To do that,transformCallbackchecked whether the member's parent was aCallExpressionand, if so, replaced the whole parent.That check is too broad. A parent can be a
CallExpressionfor two different reasons:response.json()console.log(response.code)The old code treated both the same and replaced the parent in either case.
Fix
Before, the only check was "is the parent a CallExpression?". If yes, the whole parent was replaced.
Now we check two things: the parent is a CallExpression and the member is that call's callee. Only when both hold is the whole call replaced (response.json() → response.data). Otherwise just the member itself is replaced, so whatever wraps it is kept (console.log(response.code) → console.log(response.status)).
response.json()response.dataconsole.log(response.code)response.statusconsole.log(response.status)Screenshots
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit
Bug Fixes
Tests