Skip to content

fix: parse StructuredError details array in 465 error handlers#229

Open
Leechael wants to merge 10 commits intomainfrom
fix/sdk-465-structured-error-parsing
Open

fix: parse StructuredError details array in 465 error handlers#229
Leechael wants to merge 10 commits intomainfrom
fix/sdk-465-structured-error-parsing

Conversation

@Leechael
Copy link
Copy Markdown
Collaborator

@Leechael Leechael commented Apr 3, 2026

Summary

  • Fix 465 error handlers in updateCvmEnvs, updatePrelaunchScript, and updateDockerCompose to correctly parse StructuredError format
  • Backend returns { details: [{ field: "compose_hash", value: "..." }, ...] } but SDK expected flat detail.compose_hash
  • Extract fields from details array, matching the pattern already used in patchCvm's extractDetailsMap

Test Plan

  • Update env vars on contract-owned KMS CVM — should return precondition_required instead of throwing
  • Update prelaunch script on contract-owned KMS CVM — same
  • Update docker-compose on contract-owned KMS CVM — same

Leechael added 7 commits April 2, 2026 22:38
The replicate command was using getCvmComposeConfig which calls
/cvms/{id}/compose_file — an endpoint that does not return env_pubkey.
This caused "undefined is not an object" when encrypting env vars.

Switch to safeGetCvmInfo + getEncryptPubkey, matching the approach
used by `phala env encrypt`. Remove the unused getCvmComposeConfig
helper and CvmComposeConfigResponse type.
- Add --compose-hash, --prepare-only, --commit, --token, --transaction-hash options
- Rename --teepod-id to --node-id with name resolution support
- Handle 465 responses for on-chain KMS prepare flow
- Add commit command hints to prepare-only output
- Use instance-level /cvms/{vm_uuid}/replicas endpoint
- Show Team (from source CVM workspace) and App URL in replica output
- Keep CVM UUID dashes intact (don't strip hyphens)
- Show KMS type in output
- Add 0x prefix to compose hash, app id, device id in prepare output
- Preserve ResourceError from safeGetCvmInfo so error codes (e.g.
  ERR-03-003, ERR-03-009) and suggestions are displayed
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

📋 Check Results

✨ JS SDK - Code Formatting

Show format check results
✓ No formatting issues found

🔍 JS SDK - TypeScript Type Check

Show type check output
$ tsc --noEmit

🧪 JS SDK - Test Results

Show test output
$ vitest --run --exclude '**/*.e2e.test.ts'

�[7m�[1m�[36m RUN �[39m�[22m�[27m �[36mv1.6.1�[39m �[90m/home/runner/work/phala-cloud/phala-cloud/js�[39m

 �[32m✓�[39m src/client.test.ts �[2m (�[22m�[2m45 tests�[22m�[2m)�[22m�[90m 37�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/blockchains/deploy_app_auth.test.ts �[2m (�[22m�[2m27 tests�[22m�[2m)�[22m�[90m 46�[2mms�[22m�[39m
�[90mstdout�[2m | src/actions/blockchains/add_compose_hash.test.ts�[2m > �[22m�[2maddComposeHash�[2m > �[22m�[2mStandard Version�[2m > �[22m�[2mshould add compose hash successfully with default parameters�[22m�[39m
[]

�[90mstdout�[2m | src/actions/blockchains/add_compose_hash.test.ts�[2m > �[22m�[2maddComposeHash�[2m > �[22m�[2mStandard Version�[2m > �[22m�[2mshould handle custom timeout�[22m�[39m
[]

�[90mstdout�[2m | src/actions/blockchains/add_compose_hash.test.ts�[2m > �[22m�[2maddComposeHash�[2m > �[22m�[2mStandard Version�[2m > �[22m�[2mshould use custom schema when provided�[22m�[39m
[]

�[90mstdout�[2m | src/actions/blockchains/add_compose_hash.test.ts�[2m > �[22m�[2maddComposeHash�[2m > �[22m�[2mStandard Version�[2m > �[22m�[2mshould return raw data when schema is false�[22m�[39m
[]

�[90mstdout�[2m | src/actions/blockchains/add_compose_hash.test.ts�[2m > �[22m�[2maddComposeHash�[2m > �[22m�[2mStandard Version�[2m > �[22m�[2mshould throw when custom schema validation fails�[22m�[39m
[]

�[90mstdout�[2m | src/actions/blockchains/add_compose_hash.test.ts�[2m > �[22m�[2maddComposeHash�[2m > �[22m�[2mStandard Version�[2m > �[22m�[2mshould work with wallet client authentication�[22m�[39m
[]

�[90mstdout�[2m | src/actions/blockchains/add_compose_hash.test.ts�[2m > �[22m�[2maddComposeHash�[2m > �[22m�[2mStandard Version�[2m > �[22m�[2mshould work with both clients provided�[22m�[39m
[]

�[90mstdout�[2m | src/actions/blockchains/add_compose_hash.test.ts�[2m > �[22m�[2maddComposeHash�[2m > �[22m�[2mStandard Version�[2m > �[22m�[2mshould skip prerequisite checks when configured�[22m�[39m
[]

�[90mstdout�[2m | src/actions/blockchains/add_compose_hash.test.ts�[2m > �[22m�[2maddComposeHash�[2m > �[22m�[2mStandard Version�[2m > �[22m�[2mshould use retry mechanism when enabled�[22m�[39m
[]

�[90mstdout�[2m | src/actions/blockchains/add_compose_hash.test.ts�[2m > �[22m�[2maddComposeHash�[2m > �[22m�[2mStandard Version�[2m > �[22m�[2mshould handle progress callbacks�[22m�[39m
[]

�[90mstdout�[2m | src/actions/blockchains/add_compose_hash.test.ts�[2m > �[22m�[2maddComposeHash�[2m > �[22m�[2mSafe Version�[2m > �[22m�[2mshould return success result when operation succeeds�[22m�[39m
[]

�[90mstdout�[2m | src/actions/blockchains/add_compose_hash.test.ts�[2m > �[22m�[2maddComposeHash�[2m > �[22m�[2mSafe Version�[2m > �[22m�[2mshould work with custom schema�[22m�[39m
[]

�[90mstdout�[2m | src/actions/blockchains/add_compose_hash.test.ts�[2m > �[22m�[2maddComposeHash�[2m > �[22m�[2mSafe Version�[2m > �[22m�[2mshould return raw data when schema is false�[22m�[39m
[]

�[90mstdout�[2m | src/actions/blockchains/add_compose_hash.test.ts�[2m > �[22m�[2maddComposeHash�[2m > �[22m�[2mSafe Version�[2m > �[22m�[2mshould work without parameters�[22m�[39m
[]

�[90mstdout�[2m | src/actions/blockchains/add_compose_hash.test.ts�[2m > �[22m�[2maddComposeHash�[2m > �[22m�[2mSafe Version�[2m > �[22m�[2mshould work with empty parameters object�[22m�[39m
[]

�[90mstdout�[2m | src/actions/blockchains/add_compose_hash.test.ts�[2m > �[22m�[2maddComposeHash�[2m > �[22m�[2mSchema Flexibility�[2m > �[22m�[2mshould allow extra fields in transaction receipt for forward compatibility�[22m�[39m
[]

�[90mstdout�[2m | src/actions/blockchains/add_compose_hash.test.ts�[2m > �[22m�[2maddComposeHash�[2m > �[22m�[2mSchema Flexibility�[2m > �[22m�[2mshould handle ComposeHashAdded event when present�[22m�[39m
[]

�[90mstdout�[2m | src/actions/blockchains/add_compose_hash.test.ts�[2m > �[22m�[2maddComposeHash�[2m > �[22m�[2mType Inference�[2m > �[22m�[2mshould infer correct types for default schema�[22m�[39m
[]

�[90mstdout�[2m | src/actions/blockchains/add_compose_hash.test.ts�[2m > �[22m�[2maddComposeHash�[2m > �[22m�[2mType Inference�[2m > �[22m�[2mshould infer correct types for custom schema�[22m�[39m
[]

�[90mstdout�[2m | src/actions/blockchains/add_compose_hash.test.ts�[2m > �[22m�[2maddComposeHash�[2m > �[22m�[2mType Inference�[2m > �[22m�[2mshould infer unknown type when schema is false�[22m�[39m
[]

�[90mstdout�[2m | src/actions/blockchains/add_compose_hash.test.ts�[2m > �[22m�[2maddComposeHash�[2m > �[22m�[2mSafe Version Type Inference�[2m > �[22m�[2mshould infer correct SafeResult types for default schema�[22m�[39m
[]

�[90mstdout�[2m | src/actions/blockchains/add_compose_hash.test.ts�[2m > �[22m�[2maddComposeHash�[2m > �[22m�[2mSafe Version Type Inference�[2m > �[22m�[2mshould infer correct SafeResult types for custom schema�[22m�[39m
[]

 �[32m✓�[39m src/actions/blockchains/add_compose_hash.test.ts �[2m (�[22m�[2m34 tests�[22m�[2m)�[22m�[90m 117�[2mms�[22m�[39m
 �[32m✓�[39m src/utils/errors.test.ts �[2m (�[22m�[2m30 tests�[22m�[2m)�[22m�[90m 14�[2mms�[22m�[39m
 �[32m✓�[39m src/utils/define-action.test.ts �[2m (�[22m�[2m24 tests�[22m�[2m)�[22m�[90m 26�[2mms�[22m�[39m
 �[32m✓�[39m src/utils/define-action.type.test.ts �[2m (�[22m�[2m20 tests�[22m�[2m)�[22m�[90m 22�[2mms�[22m�[39m
 �[32m✓�[39m src/types/cvm_id.test.ts �[2m (�[22m�[2m53 tests�[22m�[2m)�[22m�[90m 56�[2mms�[22m�[39m
 �[32m✓�[39m src/utils/hostname.test.ts �[2m (�[22m�[2m53 tests�[22m�[2m)�[22m�[90m 22�[2mms�[22m�[39m
 �[32m✓�[39m src/parse_dotenv.test.ts �[2m (�[22m�[2m71 tests�[22m�[2m)�[22m�[90m 22�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/get_current_user.test.ts �[2m (�[22m�[2m14 tests�[22m�[2m)�[22m�[90m 23�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/cvms/provision_cvm_compose_file_update.test.ts �[2m (�[22m�[2m29 tests�[22m�[2m)�[22m�[90m 33�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/cvms/commit_cvm_compose_file_update.test.ts �[2m (�[22m�[2m12 tests�[22m�[2m)�[22m�[90m 22�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/cvms/get_cvm_info.test.ts �[2m (�[22m�[2m11 tests�[22m�[2m)�[22m�[90m 35�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/cvms/watch_cvm_state.test.ts �[2m (�[22m�[2m6 tests�[22m�[2m)�[22m�[90m 54�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/cvms/get_cvm_compose_file.test.ts �[2m (�[22m�[2m10 tests�[22m�[2m)�[22m�[90m 30�[2mms�[22m�[39m
 �[32m✓�[39m src/version-inference.type.test.ts �[2m (�[22m�[2m17 tests�[22m�[2m)�[22m�[90m 75�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/cvms/provision_cvm.test.ts �[2m (�[22m�[2m14 tests�[22m�[2m)�[22m�[90m 22�[2mms�[22m�[39m
 �[32m✓�[39m src/client.extend.type.test.ts �[2m (�[22m�[2m8 tests�[22m�[2m)�[22m�[90m 6�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/cvms/get_cvm_list.test.ts �[2m (�[22m�[2m6 tests�[22m�[2m)�[22m�[90m 15�[2mms�[22m�[39m
 �[32m✓�[39m src/version-inference.runtime.test.ts �[2m (�[22m�[2m9 tests�[22m�[2m)�[22m�[90m 16�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/kms/get_app_env_encrypt_pubkey.test.ts �[2m (�[22m�[2m9 tests�[22m�[2m)�[22m�[90m 20�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/cvms/patch_cvm.test.ts �[2m (�[22m�[2m10 tests�[22m�[2m)�[22m�[90m 19�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/kms/get_kms_list.test.ts �[2m (�[22m�[2m8 tests�[22m�[2m)�[22m�[90m 19�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/cvms/commit_cvm_provision.test.ts �[2m (�[22m�[2m7 tests�[22m�[2m)�[22m�[90m 13�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/kms/get_kms_info.test.ts �[2m (�[22m�[2m8 tests�[22m�[2m)�[22m�[90m 22�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/cvms/get_cvm_stats.test.ts �[2m (�[22m�[2m8 tests�[22m�[2m)�[22m�[90m 21�[2mms�[22m�[39m
 �[32m✓�[39m src/utils/verify-webhook.test.ts �[2m (�[22m�[2m11 tests�[22m�[2m)�[22m�[90m 11�[2mms�[22m�[39m
 �[32m✓�[39m src/client.extend.test.ts �[2m (�[22m�[2m6 tests�[22m�[2m)�[22m�[90m 14�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/cvms/confirm_cvm_patch.test.ts �[2m (�[22m�[2m9 tests�[22m�[2m)�[22m�[90m 17�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/cvms/update_cvm_envs.test.ts �[2m (�[22m�[2m6 tests�[22m�[2m)�[22m�[90m 15�[2mms�[22m�[39m
 �[32m✓�[39m src/utils/validate-parameters.test.ts �[2m (�[22m�[2m9 tests�[22m�[2m)�[22m�[90m 14�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/get_available_nodes.test.ts �[2m (�[22m�[2m6 tests�[22m�[2m)�[22m�[90m 15�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/ssh_keys/create_ssh_key.test.ts �[2m (�[22m�[2m9 tests�[22m�[2m)�[22m�[90m 17�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/cvms/shutdown_cvm.test.ts �[2m (�[22m�[2m8 tests�[22m�[2m)�[22m�[90m 15�[2mms�[22m�[39m
 �[32m✓�[39m src/types/app_compose.test.ts �[2m (�[22m�[2m9 tests�[22m�[2m)�[22m�[90m 15�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/cvms/restart_cvm.test.ts �[2m (�[22m�[2m8 tests�[22m�[2m)�[22m�[90m 17�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/cvms/start_cvm.test.ts �[2m (�[22m�[2m8 tests�[22m�[2m)�[22m�[90m 18�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/cvms/stop_cvm.test.ts �[2m (�[22m�[2m8 tests�[22m�[2m)�[22m�[90m 13�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/cvms/get_cvm_containers_stats.test.ts �[2m (�[22m�[2m6 tests�[22m�[2m)�[22m�[90m 12�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/ssh_keys/delete_ssh_key.test.ts �[2m (�[22m�[2m8 tests�[22m�[2m)�[22m�[90m 14�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/ssh_keys/sync_github_ssh_keys.test.ts �[2m (�[22m�[2m6 tests�[22m�[2m)�[22m�[90m 12�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/cvms/delete_cvm.test.ts �[2m (�[22m�[2m8 tests�[22m�[2m)�[22m�[90m 10�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/cvms/get_cvm_state.test.ts �[2m (�[22m�[2m4 tests�[22m�[2m)�[22m�[90m 13�[2mms�[22m�[39m
 �[32m✓�[39m src/utils/as-hex.test.ts �[2m (�[22m�[2m9 tests�[22m�[2m)�[22m�[90m 7�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/cvms/refresh_cvm_instance_ids.test.ts �[2m (�[22m�[2m2 tests�[22m�[2m)�[22m�[90m 10�[2mms�[22m�[39m
 �[32m✓�[39m src/actions/cvms/refresh_cvm_instance_id.test.ts �[2m (�[22m�[2m2 tests�[22m�[2m)�[22m�[90m 7�[2mms�[22m�[39m

�[2m Test Files �[22m �[1m�[32m46 passed�[39m�[22m�[90m (46)�[39m
�[2m      Tests �[22m �[1m�[32m685 passed�[39m�[22m�[90m (685)�[39m
�[2m   Start at �[22m 12:37:14
�[2m   Duration �[22m 9.43s�[2m (transform 1.31s, setup 8ms, collect 12.49s, tests 1.07s, environment 12ms, prepare 5.70s)�[22m

📝 JS SDK - Lint Check

Show lint results
✓ No linting issues found

🌐 JS SDK - Browser Compatibility

Show browser test results

🌐 Browser Compatibility Report

Browser compatibility tests completed across:

  • ✓ Chromium
  • ✓ Firefox
  • ✓ WebKit (Safari)

The SDK has been verified to work in modern browser environments.


Check run: https://github.com/Phala-Network/phala-cloud/actions/runs/23946402732

Leechael added 3 commits April 3, 2026 20:36
The 465 error handlers in updateCvmEnvs, updatePrelaunchScript, and
updateDockerCompose expected flat fields on the error detail object
(e.g. detail.compose_hash), but the backend returns StructuredError
format with a details array:

  { details: [{ field: "compose_hash", value: "..." }, ...] }

Extract fields from the details array using the same pattern already
used in patchCvm's extractDetailsMap function. This fixes the two-phase
on-chain signing flow for contract-owned KMS CVMs.
RequestError.fromFetchError set detail to parseResult.data.detail,
which is undefined for StructuredError responses (they use 'details'
array, not 'detail'). This caused the original response body to be
lost when converting FetchError → RequestError → BusinessError.

When detail is undefined but error.data contains a StructuredError
(has error_code), fall back to using error.data as detail. This
allows parseStructuredError in parseApiError to correctly parse it
and return a ResourceError with the full structured data.

Also remove debug console.log from updateCvmEnvs.
- errors.test.ts: verify RequestError.fromFetchError preserves
  StructuredError response body as detail, and parseApiError
  produces ResourceError with correct fields
- update_cvm_envs.test.ts: verify 465 → precondition_required
  conversion with compose_hash, app_id, kms_info extraction
@Leechael Leechael force-pushed the fix/sdk-465-structured-error-parsing branch from 216da9e to c6e16ef Compare April 3, 2026 12:36
Comment on lines +231 to +241
if (composeHash && appId) {
return {
status: "precondition_required" as const,
message: (detailObj.message as string) || "Compose hash verification required",
compose_hash: composeHash as string,
app_id: appId as string,
device_id: (deviceId as string) || "",
kms_info: kmsInfo,
};
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The logic for handling 465 errors checks for composeHash and appId but not kmsInfo, leading to a schema validation failure if kmsInfo is missing from the error details.
Severity: MEDIUM

Suggested Fix

To fix the logical inconsistency, either make the kms_info field optional in the Zod schema (e.g., kms_info: KmsInfoSchema.optional()) or add a check for kmsInfo in the conditional statement to ensure it exists before proceeding (e.g., if (composeHash && appId && kmsInfo)).

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/actions/cvms/update_cvm_envs.ts#L231-L241

Potential issue: In the error handling for a 465 status, the code extracts
`composeHash`, `appId`, and `kmsInfo` from the error details. It then proceeds if
`composeHash` and `appId` are present, but it does not check for the existence of
`kmsInfo`. The resulting object is then validated against a schema where the `kms_info`
field is mandatory. If the backend returns a 465 error that includes `composeHash` and
`appId` but omits `kms_info`, the code will attempt to build an object with an undefined
`kms_info`. This will cause a Zod schema validation error, throwing an exception instead
of correctly returning the `precondition_required` status and breaking the intended
two-phase update flow.

Did we get this right? 👍 / 👎 to inform future reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant