diff --git a/.changeset/fix-396-cargo-metadata-actionable-error.md b/.changeset/fix-396-cargo-metadata-actionable-error.md new file mode 100644 index 00000000..af6af341 --- /dev/null +++ b/.changeset/fix-396-cargo-metadata-actionable-error.md @@ -0,0 +1,5 @@ +--- +"playground-cli": patch +--- + +Contract deploy now shows an actionable message when `cargo metadata` fails, instead of dumping the raw `Command failed: cargo metadata … --no-deps` command. The message explains the likely causes (missing Rust toolchain, offline git-dependency fetch, or an invalid Cargo.toml) and still surfaces cargo's own diagnostic (e.g. the offending `Cargo.toml` line) so a malformed manifest stays fixable. diff --git a/src/commands/contract.ts b/src/commands/contract.ts index 2b7b5165..86485fed 100644 --- a/src/commands/contract.ts +++ b/src/commands/contract.ts @@ -62,6 +62,7 @@ import { import { getAssetHubDescriptor, getBulletinDescriptor } from "../utils/descriptors.js"; import { onProcessShutdown } from "../utils/process-guard.js"; import { resolveSigner, type ResolvedSigner, type SignerOptions } from "../utils/signer.js"; +import { remapCargoMetadataError } from "../utils/toolchain.js"; import { runContractDeployWithUI } from "./contractDeployUi.js"; import { runContractInstallWithUI } from "./contractInstallUi.js"; @@ -296,7 +297,12 @@ async function assertCdmPackageOwnership({ registryAddress: HexString; origin: SS58String; }): Promise { - const detected = detectBuildOrder(rootDir); + let detected: ReturnType; + try { + detected = detectBuildOrder(rootDir); + } catch (err) { + throw remapCargoMetadataError(err); + } const packageNames = [ ...new Set( detected.contracts diff --git a/src/commands/contractDeployUi.test.ts b/src/commands/contractDeployUi.test.ts new file mode 100644 index 00000000..35f6c626 --- /dev/null +++ b/src/commands/contractDeployUi.test.ts @@ -0,0 +1,39 @@ +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { describe, expect, it, vi } from "vitest"; + +// Make detectBuildOrder (which shells out to `cargo metadata`) throw, so we can +// assert the call site applies the remap (#396), not just that the helper does. +vi.mock("@parity/cdm-builder", () => ({ + detectBuildOrder: () => { + throw new Error( + 'Command failed: cargo metadata --format-version 1 --manifest-path "/p/Cargo.toml" --no-deps\n' + + "error: could not parse manifest\n", + ); + }, + deployContracts: vi.fn(), +})); + +import { CARGO_METADATA_MESSAGE } from "../utils/toolchain.js"; +import { precomputeContractDeployDisplay } from "./contractDeployUi.js"; + +describe("precomputeContractDeployDisplay cargo metadata remap (#396)", () => { + it("remaps a detectBuildOrder cargo metadata failure to the actionable message", () => { + expect(() => precomputeContractDeployDisplay("/p", undefined)).toThrow( + CARGO_METADATA_MESSAGE, + ); + }); +}); diff --git a/src/commands/contractDeployUi.tsx b/src/commands/contractDeployUi.tsx index a22befff..4233e65d 100644 --- a/src/commands/contractDeployUi.tsx +++ b/src/commands/contractDeployUi.tsx @@ -22,6 +22,7 @@ import { type DeploySummary, } from "@parity/cdm-builder"; import { getNetworkLabel } from "../config.js"; +import { remapCargoMetadataError } from "../utils/toolchain.js"; import { COLOR, Callout, @@ -124,7 +125,12 @@ export function precomputeContractDeployDisplay( rootDir: string, contracts: string[] | undefined, ): ContractDeployDisplay { - const order = detectBuildOrder(rootDir, contracts); + let order: ReturnType; + try { + order = detectBuildOrder(rootDir, contracts); + } catch (err) { + throw remapCargoMetadataError(err); + } const displayNames = new Map(); for (const contract of order.contracts) { displayNames.set( diff --git a/src/utils/toolchain.test.ts b/src/utils/toolchain.test.ts index 368c0285..d266fdb6 100644 --- a/src/utils/toolchain.test.ts +++ b/src/utils/toolchain.test.ts @@ -14,7 +14,15 @@ // limitations under the License. import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import { hasCargoPvmContract, isIpfsMigrationError, prependPath, TOOL_STEPS } from "./toolchain.js"; +import { + CARGO_METADATA_MESSAGE, + hasCargoPvmContract, + isCargoMetadataError, + isIpfsMigrationError, + prependPath, + remapCargoMetadataError, + TOOL_STEPS, +} from "./toolchain.js"; describe("prependPath", () => { let originalPath: string | undefined; @@ -157,3 +165,45 @@ describe("isIpfsMigrationError", () => { expect(isIpfsMigrationError(new Error("database needs migration"))).toBe(false); }); }); + +describe("cargo metadata error remap (#396)", () => { + // The shape cdm-builder's getCargoMetadata rethrows: execSync's bare + // "Command failed: cargo metadata …" with cargo's stderr appended. + const raw = new Error( + 'Command failed: cargo metadata --format-version 1 --manifest-path "/p/Cargo.toml" --no-deps\n' + + "error: key with no value, expected `=`\n", + ); + + it("classifies cdm-builder's raw cargo metadata failure", () => { + expect(isCargoMetadataError(raw)).toBe(true); + }); + + it("remaps to the actionable message while keeping cargo's own diagnostic", () => { + const remapped = remapCargoMetadataError(raw) as Error; + expect(remapped).toBeInstanceOf(Error); + expect(remapped.message.startsWith(CARGO_METADATA_MESSAGE)).toBe(true); + // Must NOT drop cargo's specifics, or a malformed Cargo.toml loses the + // detail that lets the user fix it. + expect(remapped.message).toContain("key with no value"); + // The raw error stays reachable for diagnostics — wrap, never discard. + expect(remapped.cause).toBe(raw); + }); + + it("surfaces cargo's clean stderr over the noisy 'Command failed' echo", () => { + // execSync puts the clean diagnostic on `.stderr` and the command echo + // on `.message`; prefer stderr so the user sees file/line, not the command. + const withStderr = Object.assign( + new Error('Command failed: cargo metadata --manifest-path "/p/Cargo.toml" --no-deps'), + { stderr: "error: key with no value, expected `=`\n --> Cargo.toml:21:6\n" }, + ); + const remapped = remapCargoMetadataError(withStderr) as Error; + expect(remapped.message).toContain("Cargo.toml:21:6"); + expect(remapped.message).not.toContain("--manifest-path"); + }); + + it("passes unrelated deploy errors through unchanged", () => { + const other = new Error("AncientBirthBlock: chunk rejected"); + expect(remapCargoMetadataError(other)).toBe(other); + expect(isCargoMetadataError(other)).toBe(false); + }); +}); diff --git a/src/utils/toolchain.ts b/src/utils/toolchain.ts index f5946cdc..11149a89 100644 --- a/src/utils/toolchain.ts +++ b/src/utils/toolchain.ts @@ -101,6 +101,65 @@ export function isIpfsMigrationError(err: unknown): boolean { return /repo needs migration/i.test(err instanceof Error ? err.message : String(err)); } +/** + * True when an error is cdm-builder's raw `cargo metadata` failure. cdm-builder + * (`getCargoMetadata`) shells out to `cargo metadata --format-version 1 + * --manifest-path <…> --no-deps` and rethrows execSync's bare `Command failed: + * cargo metadata …` on any non-zero exit — a missing/partial Rust toolchain, an + * unfetched git dependency (the CDM / pvm-contract crates resolve over the + * network), or a malformed Cargo.toml. We match the stable `cargo metadata` + * fragment case-insensitively so the surrounding `Command failed: …` prefix and + * cargo's stderr can vary. + * + * Single source of truth for the marker, shared with the deploy-time remap + * (`contract.ts` / `contractDeployUi.tsx` via `remapCargoMetadataError`). + */ +export function isCargoMetadataError(err: unknown): boolean { + return /cargo metadata/i.test(err instanceof Error ? err.message : String(err)); +} + +/** + * Actionable replacement for cdm-builder's raw `Command failed: cargo metadata …` + * dump (issue #396). The failure means `cargo metadata` could not read the + * contract workspace — usually a missing Rust toolchain, an unfetched git + * dependency (the CDM / pvm-contract crates resolve over the network), or a + * malformed Cargo.toml. We point at the concrete remedies instead of echoing + * the bare command. + */ +export const CARGO_METADATA_MESSAGE = + "Couldn't read the contract workspace: `cargo metadata` failed. Check that Rust is " + + "installed (run `playground login` to set up the toolchain), that you're online so the " + + "CDM/pvm-contract git dependencies can be fetched, and that each contract's Cargo.toml is " + + "valid, then try again."; + +/** + * Pull cargo's own diagnostic out of cdm-builder's execSync failure so the + * actionable message keeps the specifics (e.g. `Cargo.toml:21:6 key with no + * value`) that actually let the user fix a malformed manifest. Prefer `.stderr` + * (cargo's clean output) over `.message`, which prefixes the noisy + * `Command failed: cargo metadata …` command echo. + */ +function cargoErrorDetail(err: unknown): string { + if (typeof err !== "object" || err === null) return ""; + const e = err as { stderr?: unknown; message?: unknown }; + if (typeof e.stderr === "string" && e.stderr.trim()) return e.stderr.trim(); + return typeof e.message === "string" ? e.message.trim() : ""; +} + +/** + * Replace cdm-builder's cryptic `Command failed: cargo metadata …` with an + * actionable instruction that still surfaces cargo's own diagnostic (so a + * malformed Cargo.toml keeps its file/line). Non-cargo-metadata errors pass + * through unchanged; the raw error stays reachable via `.cause`. + */ +export function remapCargoMetadataError(err: unknown): unknown { + if (!isCargoMetadataError(err)) return err; + const detail = cargoErrorDetail(err); + return new Error(detail ? `${CARGO_METADATA_MESSAGE}\n\n${detail}` : CARGO_METADATA_MESSAGE, { + cause: err, + }); +} + /** * Detect a stale Kubo repo that the installed `ipfs` binary refuses to use * until it's migrated to the current on-disk format. Kubo stamps `~/.ipfs`