fix(deploy): actionable error when cargo metadata fails (#396)

.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.

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<void> {
-    const detected = detectBuildOrder(rootDir);
+    let detected: ReturnType<typeof detectBuildOrder>;
+    try {
+        detected = detectBuildOrder(rootDir);
+    } catch (err) {
+        throw remapCargoMetadataError(err);
+    }
     const packageNames = [
         ...new Set(
             detected.contracts

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,
+        );
+    });
+});

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<typeof detectBuildOrder>;
+    try {
+        order = detectBuildOrder(rootDir, contracts);
+    } catch (err) {
+        throw remapCargoMetadataError(err);
+    }
     const displayNames = new Map<string, string>();
     for (const contract of order.contracts) {
         displayNames.set(

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);
+    });
+});

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`