fix(runner): stop misdiagnosing a broken package.json as an unpublished version (DEV-2872)

runner/apps/authoring/src/App.tsx

@@ -71,6 +71,7 @@ import { monitorDemos, reportDemoEvent, reportError, reportingEnabled, Sentry }
 import { isMonitorPayload } from "@handsontable/demo-runtime/monitor";
 import { tier1Report } from "./tier1Report.js";
 import { isOpaqueNetworkFailure } from "./fetchFailure.js";
+import { describeDependencyFailure } from "./dependencyFailure.js";
 import {
   readFetchDiagnostics,
   apiBaseOrigin,
@@ -182,7 +183,12 @@ function docsPageUrl(framework: string, permalink: string): string {
 }
 
 /** Turn a raw runtime error into a message that explains container prerequisites. */
-function describeRuntimeError(e: unknown, engine: string, version: string): string {
+function describeRuntimeError(
+  e: unknown,
+  engine: string,
+  version: string,
+  packageJson: string | undefined,
+): string {
   // A boot-script failure explains itself: a one-line cause, with the recent boot
   // output beside it on the error object. Compose the two here — the error card
   // renders `errorMessage` and nothing else, so this is the only place a user ever
@@ -217,12 +223,15 @@ function describeRuntimeError(e: unknown, engine: string, version: string): stri
     return "This example runs on the container engine, which needs the demo server (Cloudflare Sandbox). It isn't reachable here — run the local API worker (requires Docker) or open this example on the deployed demos.handsontable.com.";
   }
   // Sandpack's own bundler message for an unresolved dependency reads like a
-  // transient hiccup worth retrying ("please try again in a couple
-  // seconds") — misleading when the actual cause is a pinned Handsontable
-  // version that was never published, which no amount of retrying fixes.
-  if (engine === "sandpack" && /could not fetch dependencies/i.test(msg)) {
-    return `Handsontable ${version} could not be fetched. Check that this exact version is published on npm.`;
-  }
+  // transient hiccup worth retrying ("please try again in a couple seconds") —
+  // misleading when the actual cause is a pinned Handsontable version that was
+  // never published, which no amount of retrying fixes. But that same wording is
+  // ALSO what Sandpack says when the authored `/package.json` doesn't parse
+  // (DEV-2872, Sentry DEMOS-15/DEMOS-85) — see `dependencyFailure.ts` for how the
+  // two are told apart and why the discriminator is our own file state, not the
+  // bundler's text.
+  const dependencyFailure = describeDependencyFailure({ engine, message: msg, version, packageJson });
+  if (dependencyFailure) return dependencyFailure;
   return msg;
 }
 
@@ -2340,7 +2349,7 @@ function Authoring({
     runtime.onError((e) => {
       if (cancelled) return;
       setStatus("error");
-      setErrorMessage(describeRuntimeError(e, entry.engine, v.value.ref));
+      setErrorMessage(describeRuntimeError(e, entry.engine, v.value.ref, filesRef.current["/package.json"]));
       reportRuntimeError(e, entry.engine, entry.framework);
     });
     runtimeRef.current = runtime;
@@ -2358,7 +2367,7 @@ function Authoring({
       .catch((e: unknown) => {
         if (!cancelled) {
           setStatus("error");
-          setErrorMessage(describeRuntimeError(e, entry.engine, v.value.ref));
+          setErrorMessage(describeRuntimeError(e, entry.engine, v.value.ref, filesRef.current["/package.json"]));
         }
         // Reported even when cancelled: a session the pool refused still failed,
         // and the unmount that set `cancelled` is often the user giving up on it.

runner/apps/authoring/src/dependencyFailure.ts

@@ -0,0 +1,113 @@
+/**
+ * Which of two causes put Sandpack's "could not fetch dependencies" message in front
+ * of a visitor (DEV-2872, Sentry DEMOS-15 / DEMOS-85).
+ *
+ * Split out of `App.tsx` for the same reason as `tier1Report.ts`, `fetchFailure.ts`
+ * and `sessionDiagnostics.ts`: that file pulls `@sentry/react` and reads
+ * `import.meta.env`, so `node --test` cannot import it, and nothing in it can be
+ * pinned by a unit test. Keep this module import-free — the discriminator below is
+ * the whole of what it decides, and `pipeline/dependency-failure.test.mjs` imports it
+ * as source under `--experimental-strip-types`. Do not let this file grow imports.
+ *
+ * WHY NOW. DEV-2855 guarded `buildSetup` so an unparseable `/package.json` no longer
+ * aborts `mount()`. That fix is correct and stays — before it, the preview died and
+ * every later keystroke was silently swallowed until "Restart preview". But it changed
+ * *which* message reaches `describeRuntimeError`: the mount now survives, the bundler
+ * fails trying to resolve dependencies out of a manifest it cannot parse, and that
+ * failure carries the exact same "could not fetch dependencies" wording Sandpack uses
+ * for an unpublished/unresolvable Handsontable version. Both populations now land on
+ * one branch in `App.tsx` that used to have only one cause.
+ *
+ * THE DISCRIMINATOR IS OUR OWN FILE STATE, NOT THIRD-PARTY TEXT. The two causes are
+ * told apart by whether the authored `/package.json` parses as JSON — not by matching
+ * on `Cannot read properties of null (reading 'match')`, which is Sandpack's bundler
+ * choking on a manifest with no resolvable `dependencies` object. That wording is
+ * undocumented upstream behaviour; keying on it would stop discriminating the moment
+ * the bundler is bumped and phrases its own internal failure differently. Reading our
+ * own file's parseability is stable regardless of how the bundler happens to fail on
+ * it. The branch *entry* — `/could not fetch dependencies/i` — still keys on Sandpack's
+ * wording, which is fine: that regex only gates onto our decision below and fails open
+ * (returns the bundler's own message, via `App.tsx`'s existing `return msg`) rather
+ * than fabricating a wrong answer.
+ *
+ * WHY THE PARSE DETAIL IS ASSERTED LOOSELY. `jsonSyntaxError` below hands back
+ * whatever `JSON.parse` throws for `.message`, verbatim. That wording is
+ * engine-specific — `fetchFailure.ts`'s header documents the same divergence for
+ * Chrome/Firefox/Safari network error strings — so a test (or this docblock) must not
+ * assert a particular V8 sentence; the shape (a non-empty detail string) is the
+ * contract, not the words.
+ *
+ * RESIDUAL, OUT OF SCOPE. A syntactically *valid* `/package.json` pinned to a bogus
+ * non-Handsontable dependency still fails the same way and still gets the npm
+ * sentence below — that manifest parses fine, so `jsonSyntaxError` returns `null` and
+ * this module falls through to the version-not-published wording. Fixing that would
+ * require keying on the bundler's own error text for "this specific package doesn't
+ * exist", which is exactly the third-party-text dependency this module exists to
+ * avoid. Left alone deliberately.
+ */
+
+export interface DependencyFailureFacts {
+  /** Which preview engine raised the error. Only "sandpack" (Tier 1) is ever this
+   *  module's branch — the container engine (Tier 2) has its own DEV-2538/DEV-2553
+   *  contract and must never be perturbed here. */
+  engine: string;
+  /** `e.message`, exactly as the shell received it from the runtime — the bounded
+   *  `show-error` text, not a Sentry-rendered cause chain (that tail is `Error`'s own
+   *  `cause` formatting and never reaches this module). */
+  message: string;
+  /** The pinned Handsontable version ref, for the npm sentence. */
+  version: string;
+  /** The authored `/package.json` text at error time, or `undefined` if the file
+   *  doesn't exist in this workspace. */
+  packageJson: string | undefined;
+}
+
+/**
+ * `JSON.parse(raw)` and report why it failed, or `null` if it parses.
+ * The message is whatever the engine's `JSON.parse` throws — see the docblock above
+ * for why that wording is never asserted verbatim.
+ */
+function jsonSyntaxError(raw: string): string | null {
+  try {
+    JSON.parse(raw);
+    return null;
+  } catch (e) {
+    return e instanceof Error ? e.message : String(e);
+  }
+}
+
+/**
+ * Decide how a Sandpack "could not fetch dependencies" failure should be explained,
+ * or that this isn't that failure at all.
+ *
+ * `null` means "not my branch" — the caller (`describeRuntimeError` in `App.tsx`)
+ * falls through to its existing `return msg`, the bundler's own message verbatim.
+ */
+export function describeDependencyFailure(facts: DependencyFailureFacts): string | null {
+  if (facts.engine !== "sandpack") return null;
+  if (!/could not fetch dependencies/i.test(facts.message)) return null;
+
+  if (facts.packageJson !== undefined) {
+    const detail = jsonSyntaxError(facts.packageJson);
+    if (detail) {
+      // The sentence deliberately stops at "fix the file" and does NOT promise
+      // that the preview recovers on its own. Our half of that is verified —
+      // `sandpack.ts`'s `emitReady()` fires on any bundler `done` without a
+      // compilation error, and the editor's `writeFile` is not gated on preview
+      // status, so a corrected manifest does reach the live runtime. What is NOT
+      // verifiable from this repo is whether Sandpack re-resolves dependencies
+      // from a changed `/package.json` mid-session or only at mount. Promising
+      // self-recovery on an unverified third-party behaviour would replace one
+      // false statement (DEV-2872's npm sentence) with another, which is the
+      // whole thing this module exists to stop. If someone confirms mid-session
+      // re-resolution against a live bundler, extend the sentence then.
+      return `/package.json is not valid JSON, so this demo's dependencies could not be installed: ${detail}. Fix the file to continue.`;
+    }
+  }
+
+  // Either the manifest is undefined (deliberate status-quo preservation — DEV-2872
+  // does not change this case) or it parses fine, in which case the real cause is
+  // Sandpack's own: an unresolvable/unpublished version. Byte-identical to the
+  // sentence `App.tsx` used before this module existed, so no behaviour changes here.
+  return `Handsontable ${facts.version} could not be fetched. Check that this exact version is published on npm.`;
+}

runner/pipeline/dependency-failure.test.mjs

@@ -0,0 +1,122 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import { describeDependencyFailure } from "../apps/authoring/src/dependencyFailure.ts";
+
+// DEV-2872 / Sentry DEMOS-15 / DEMOS-85. DEV-2855 guarded `buildSetup` so an
+// unparseable `/package.json` no longer aborts `mount()` — the preview survives, but
+// the bundler then fails trying to resolve dependencies out of a manifest it can't
+// read, and that failure carries the SAME "could not fetch dependencies" wording
+// Sandpack uses for an unpublished/unresolvable Handsontable version. This module is
+// what tells the two apart, by checking whether our own `/package.json` actually
+// parses — not by matching the bundler's internal failure text.
+
+// FIXTURE CORRECTION (load-bearing): the ClickUp ticket quotes Sentry's
+// `error.value` as
+//   "...(reading 'match'), Tier-1 compile failed"
+// That trailing clause is NOT part of `e.message` as `describeRuntimeError` receives
+// it. `App.tsx` reports the Tier-1 case as
+// `new Error(report.synthesizeAs.message, { cause: e })`, so Sentry is rendering the
+// cause chain — "Tier-1 compile failed" is `COMPILE_TITLE`, the synthesized error's
+// own message, not the runtime error's. What actually reaches
+// `describeRuntimeError`/`describeDependencyFailure` is the bounded `show-error`
+// text below, WITHOUT that tail. The regex this module gates on matches either way,
+// which is exactly why using the verbatim, untailed string matters here: a fixture
+// with the tail baked in would still pass even if the module accidentally depended
+// on text that never reaches it in production.
+const MANIFEST_BUNDLER_MESSAGE =
+  "Could not fetch dependencies, please try again in a couple seconds: Cannot read properties of null (reading 'match')";
+
+// The other real production wording for the actual unpublished-version case — pinned
+// already by pipeline/sandpack-reload.test.mjs's
+// "DEV-2550: the dependency-fetch message reaches describeRuntimeError intact" test.
+const VERSION_BUNDLER_MESSAGE =
+  "Could not fetch dependencies, please try again in a couple seconds: request to https://registry.npmjs.org/handsontable failed";
+
+// A mid-keystroke broken manifest: a trailing comma inside `dependencies`, exactly
+// the shape a visitor produces while still typing.
+const BROKEN_PACKAGE_JSON = `{
+  "name": "demo",
+  "dependencies": {
+    "handsontable": "14.0.0",
+  }
+}
+`;
+
+const VALID_PACKAGE_JSON = `{
+  "name": "demo",
+  "dependencies": {
+    "handsontable": "99.0.0"
+  }
+}
+`;
+
+const facts = (over = {}) => ({
+  engine: "sandpack",
+  message: MANIFEST_BUNDLER_MESSAGE,
+  version: "99.0.0",
+  packageJson: BROKEN_PACKAGE_JSON,
+  ...over,
+});
+
+// Fix-prover: with the manifest branch removed (revert baseline), this fails —
+// the module would return the npm sentence for a broken manifest instead of naming
+// the JSON problem.
+test("a broken manifest gets the JSON sentence, not the npm one (DEV-2872 fix-prover)", () => {
+  const msg = describeDependencyFailure(facts());
+  assert.match(msg, /is not valid JSON/);
+  assert.doesNotMatch(msg, /published on npm/);
+});
+
+// Fix-prover: the parse detail must travel into the message. Computed by the TEST's
+// own try/catch over the SAME fixture, never a hardcoded V8 string — JSON parse
+// wording is engine-specific (see dependencyFailure.ts's docblock, and
+// fetchFailure.ts's documented Chrome/Firefox/Safari divergence for the same kind of
+// caution).
+test("the JSON.parse detail travels into the message (DEV-2872 fix-prover)", () => {
+  let detail;
+  try {
+    JSON.parse(BROKEN_PACKAGE_JSON);
+    assert.fail("fixture must actually be invalid JSON");
+  } catch (e) {
+    detail = e.message;
+  }
+  const msg = describeDependencyFailure(facts());
+  assert.ok(detail.length > 0, "the fixture must produce a real parse error");
+  assert.ok(msg.includes(detail), `expected the message to include the parse detail: ${detail}`);
+});
+
+// Guard: a valid manifest plus the unpublished-version wording still gets the npm
+// sentence, byte-for-byte — this module must not perturb the case DEV-2872 says to
+// preserve.
+test("a valid manifest with the unpublished-version wording keeps the npm sentence (guard)", () => {
+  const msg = describeDependencyFailure(
+    facts({ message: VERSION_BUNDLER_MESSAGE, packageJson: VALID_PACKAGE_JSON, version: "99.0.0" }),
+  );
+  assert.equal(msg, "Handsontable 99.0.0 could not be fetched. Check that this exact version is published on npm.");
+});
+
+// Guard: a message that isn't the "could not fetch dependencies" wording at all is
+// not this module's branch, regardless of manifest state — the entry gate must still
+// key on Sandpack's own wording.
+test("a broken manifest with an unrelated (babel code-frame) message is not this branch (guard)", () => {
+  const msg = describeDependencyFailure(
+    facts({ message: "/src/main.ts: Unexpected token (3:1)" }),
+  );
+  assert.equal(msg, null);
+});
+
+// Guard: the container engine (Tier 2, DEV-2538/DEV-2553/DEMOS-9) must never be
+// perturbed by this module, even with a matching message and a broken manifest.
+// Pinned separately from pipeline/session-start-failure.test.mjs, which owns that
+// contract's other end.
+test("the container engine is never this branch (guard, DEV-2538 contract)", () => {
+  const msg = describeDependencyFailure(facts({ engine: "container" }));
+  assert.equal(msg, null);
+});
+
+// Guard: an absent /package.json (deliberate status-quo preservation) keeps the npm
+// sentence, same as before this module existed.
+test("an absent /package.json keeps the npm sentence (guard)", () => {
+  const msg = describeDependencyFailure(facts({ packageJson: undefined }));
+  assert.equal(msg, "Handsontable 99.0.0 could not be fetched. Check that this exact version is published on npm.");
+});

runner/pipeline/sandpack-reload.test.mjs

@@ -353,10 +353,10 @@ test("DEV-2550: a short message is reported byte-identical", () => {
 });
 
 test("DEV-2550: the dependency-fetch message reaches describeRuntimeError intact", () => {
-  // App.tsx's `/could not fetch dependencies/i` branch rewrites this into the
-  // "check that this version is published on npm" card. The phrase leads the
-  // bundler's message, so the cap cannot reach it — pinned here rather than
-  // left to inspection.
+  // `describeDependencyFailure` (apps/authoring/src/dependencyFailure.ts) rewrites
+  // this into the "check that this version is published on npm" card. The phrase
+  // leads the bundler's message, so the cap cannot reach it — pinned here rather
+  // than left to inspection.
   const raw = "Could not fetch dependencies, please try again in a couple seconds: request to https://registry.npmjs.org/handsontable failed";
   assert.equal(showError(raw).message, raw);
   assert.match(showError(raw + " " + "x".repeat(9000)).message, /^could not fetch dependencies/i);