Closed
Conversation
- CVMInfo: add InProgress, Endpoints, PublicURLs, DiskSize, EncryptedEnvPubkey - SSHKey: add Fingerprint, KeyType, Source, UpdatedAt - UserInfo/CreditsInfo: use pointer types for nullable field handling - ProvisionCVMRequest: add SSHAuthorizedKeys, CustomAppID, Nonce, StorageFS - CommitCVMProvisionRequest: add EncryptedEnv, EnvKeys - ProvisionComposeUpdateRequest: add Name, visibility flags, UpdateEnvVars
Update terraform submodule reference to refactor/use-go-sdk branch which includes CI workflow changes for Go SDK checkout.
Support for backend PR #1041 which made POST /cvms idempotent. Go SDK: - Add error_codes.go with all 30 error code constants (Module 01-06) - Extend APIError with Details, Suggestions, Links fields - Add IsConflict(), IsStructured(), HasErrorCode(), FormatError() - IsRetryable() now returns false for 409 with ErrorCode (deterministic) - Parse structured error fields from API responses in request.go JS SDK: - Add utils/error_codes.ts with ErrorCodes const and ErrorCode type - Add isConflictError() and isAppIdConflictError() helpers - Document idempotency behavior on commitCvmProvision Python SDK: - Add error_codes.py with all error code constants - Add ConflictError for transient 409 (no error_code) - Dispatch 409+error_code to ResourceError, plain 409 to ConflictError - Document idempotency behavior on commit_cvm_provision Terraform: - Use Go SDK structured errors for diagnostic messages - Add diagnosticFromAPIError() helper
Add PublicLogs, PublicSysinfo, PublicTcbinfo, SecureTime, StorageFS fields to ComposeFile so provision requests include all compose settings.
Remove Content-Type header when body is FormData so the browser automatically sets the multipart/form-data boundary.
Contributor
📋 Check Results✨ JS SDK - Code FormattingShow format check results🔍 JS SDK - TypeScript Type CheckShow type check output🧪 JS SDK - Test ResultsShow test output📝 JS SDK - Lint CheckShow lint results🌐 JS SDK - Browser CompatibilityShow browser test results🌐 Browser Compatibility ReportBrowser compatibility tests completed across:
The SDK has been verified to work in modern browser environments. Check run: https://github.com/Phala-Network/phala-cloud/actions/runs/23276478881 |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
+149
to
+152
| if (h) { | ||
| delete h["Content-Type"]; | ||
| delete h["content-type"]; | ||
| } |
There was a problem hiding this comment.
Bug: The code incorrectly uses the delete operator on a Headers object to remove the Content-Type header. This will silently fail, breaking FormData requests.
Severity: CRITICAL
Suggested Fix
Check if options.headers is an instance of the Headers object. If it is, use the h.delete('Content-Type') method to remove the header. Otherwise, keep the existing delete operator as a fallback for plain objects.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: js/src/client.ts#L149-L152
Potential issue: The code attempts to remove the `Content-Type` header for `FormData`
requests using the `delete` operator. However, the `ofetch` library version used in the
project provides `options.headers` as a native `Headers` object, not a plain object. The
`delete` operator silently fails on `Headers` objects. Consequently, the `Content-Type:
application/json` header will remain on `FormData` requests, causing them to fail
because the correct multipart boundary will not be set by the browser.
Did we get this right? 👍 / 👎 to inform future reviews.
Collaborator
Author
|
Replaced by clean PR based on main |
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.
Summary
Content-Typeheader when body isFormDataso the browser automatically setsmultipart/form-datawith the correct boundaryTest plan