fix: cloud sdk#451
Conversation
![hyperspace-insights[bot]](https://avatars.githubusercontent.com/u/2531208?s=60&v=4){kind=small}
There was a problem hiding this comment.
The switch from direct assignment to Object.defineProperty is likely the intended fix (e.g., to patch a read-only exported property), but both calls are missing writable: true and configurable: true, which silently freezes those properties and can break re-patching or tests.
PR Bot Information
Version: 1.26.14
- Correlation ID:
6bf1b950-76e8-11f1-877b-299e7adc465b - LLM:
anthropic--claude-4.6-sonnet - Event Trigger:
pull_request.opened - File Content Strategy: Full file content
| } | ||
| }) | ||
| cloudSDK.executeHttpRequestWithOrigin = wrap(_executeWithOrigin, { | ||
| Object.defineProperty(cloudSDK, 'executeHttpRequest', { value: _executeHttpRequest }) |
There was a problem hiding this comment.
Bug: Object.defineProperty without writable: true or configurable: true defaults both to false, making the property non-writable and non-configurable. This is a stricter result than the original simple assignment, and will silently fail (or throw in strict mode) if any downstream code tries to reassign or redefine the property (e.g. in tests or repeated patching).
| Object.defineProperty(cloudSDK, 'executeHttpRequest', { value: _executeHttpRequest }) | |
| Object.defineProperty(cloudSDK, 'executeHttpRequest', { value: _executeHttpRequest, writable: true, configurable: true }) |
Double-check suggestion before committing. Edit this comment for amendments.
Please provide feedback on the review comment by checking the appropriate box:
- 🌟 Awesome comment, a human might have missed that.
- ✅ Helpful comment
- 🤷 Neutral
- ❌ This comment is not helpful
| }) | ||
| } | ||
| }) | ||
| Object.defineProperty(cloudSDK, 'executeHttpRequestWithOrigin', { value: _executeHttpRequestWithOrigin }) |
There was a problem hiding this comment.
Bug: Same issue — writable and configurable default to false, making executeHttpRequestWithOrigin frozen on the module export.
| Object.defineProperty(cloudSDK, 'executeHttpRequestWithOrigin', { value: _executeHttpRequestWithOrigin }) | |
| Object.defineProperty(cloudSDK, 'executeHttpRequestWithOrigin', { value: _executeHttpRequestWithOrigin, writable: true, configurable: true }) |
Double-check suggestion before committing. Edit this comment for amendments.
Please provide feedback on the review comment by checking the appropriate box:
- 🌟 Awesome comment, a human might have missed that.
- ✅ Helpful comment
- 🤷 Neutral
- ❌ This comment is not helpful
1 participant
Fix Cloud SDK HTTP Request Tracing via
Object.definePropertyBug Fix
🐛 Fixes an issue with how the Cloud SDK HTTP client functions are patched for tracing. Previously, the wrapped
executeHttpRequestandexecuteHttpRequestWithOriginfunctions were assigned directly to thecloudSDKobject (e.g.,cloudSDK.executeHttpRequest = wrap(...)). This approach fails when the property is non-writable or non-configurable on the module exports. The fix usesObject.definePropertyto properly override these functions, ensuring the tracing wrappers are applied correctly.Changes
lib/tracing/cloud_sdk.js: Replaced direct property assignment (cloudSDK.executeHttpRequest = ...) withObject.defineProperty(cloudSDK, 'executeHttpRequest', { value: ... })for bothexecuteHttpRequestandexecuteHttpRequestWithOrigin. Also includes minor code style cleanup (reformatted multi-linetrace(...)calls for consistency).PR Bot Information
Version:
1.26.14anthropic--claude-4.6-sonnet6bf1b950-76e8-11f1-877b-299e7adc465bpull_request.opened