Merge pull request #480 from dappnode/fix/detect-unregistered-package-with-current-ethers

Detect an unregistered package with current ethers

Author: Marketen

src/tasks/generatePublishTxs/index.ts

@@ -12,6 +12,7 @@ import repoAbi from "../../contracts/RepoAbi.json" with { type: "json" };
 import { VerbosityOptions } from "../../commands/build/types.js";
 import { getRegistryAddressFromDnpName } from "./getRegistryAddressFromEns.js";
 import { Repo } from "./types.js";
+import { isPackageNotRegisteredError } from "../../utils/isPackageNotRegisteredError.js";
 
 const isZeroAddress = (address: string): boolean => parseInt(address) === 0;
 
@@ -145,7 +146,8 @@ async function getRepoContractIfExists({
   try {
     return await apm.getRepoContract(ensName);
   } catch (e) {
-    if (e.message.includes("Could not resolve name")) return null;
+    // No repository yet: the caller builds a newRepoWithVersion transaction.
+    if (isPackageNotRegisteredError(e)) return null;
     else throw e;
   }
 }

src/tasks/publish/subtasks/getFetchApmVersionsTask.ts

@@ -2,6 +2,7 @@ import { ListrTask } from "listr";
 import { PackageToBuildProps, ListrContextPublish, ReleaseType } from "../../../types.js";
 import { getNextVersionFromApm } from "../../../utils/versions/getNextVersionFromApm.js";
 import { Manifest } from "@dappnode/types";
+import { isPackageNotRegisteredError } from "../../../utils/isPackageNotRegisteredError.js";
 
 export function getFetchNextVersionsFromApmTask({
   releaseType,
@@ -53,7 +54,9 @@ export async function getNextPackageVersion({
       ensName: name
     });
   } catch (e) {
-    if (e.message.includes("NOREPO")) return version;
+    // Publishing this package for the first time: there is no repository to
+    // read a current version from, so start from the manifest version.
+    if (isPackageNotRegisteredError(e)) return version;
     else throw e;
   }
 }

src/utils/isPackageNotRegisteredError.ts

@@ -0,0 +1,32 @@
+/**
+ * True when an APM lookup failed because the package has no repository on the
+ * registry yet, rather than because something actually went wrong.
+ *
+ * This is the normal state when publishing a package for the first time, and
+ * callers use it to fall back to the version declared in the manifest.
+ *
+ * The condition has to be recognised from the error message because it surfaces
+ * differently depending on the ethers version resolving the ENS name:
+ *
+ * - `NOREPO` is raised by @dappnode/toolkit when the name resolves but carries
+ *   no repository.
+ * - `Could not resolve name` is what ethers v6 reported for an unresolvable
+ *   name in earlier releases.
+ * - `ResolverNotFound` is the ENS custom error current ethers decodes and
+ *   raises instead. Without it, a first publish fails with
+ *   `execution reverted: ResolverNotFound(bytes)`.
+ *
+ * Errors that merely reverted for some other reason are deliberately not
+ * matched, so a genuine registry failure is never mistaken for an unpublished
+ * package.
+ */
+export function isPackageNotRegisteredError(error: unknown): boolean {
+  const message =
+    error instanceof Error ? error.message : typeof error === "string" ? error : "";
+
+  return (
+    message.includes("NOREPO") ||
+    message.includes("Could not resolve name") ||
+    message.includes("ResolverNotFound")
+  );
+}

test/utils/isPackageNotRegisteredError.test.ts

@@ -0,0 +1,46 @@
+import { expect } from "chai";
+import { isPackageNotRegisteredError } from "../../src/utils/isPackageNotRegisteredError.js";
+
+describe("isPackageNotRegisteredError", () => {
+  // Each string is what an APM lookup for an unpublished package actually
+  // produced, so the first publish must keep working across ethers versions.
+  const notRegistered: { name: string; message: string }[] = [
+    {
+      name: "toolkit reports no repository",
+      message: "Could not resolve name foo.dnp.dappnode.eth: NOREPO"
+    },
+    {
+      name: "ethers v6 before the custom-error decoding",
+      message: "Could not resolve name foo.dnp.dappnode.eth"
+    },
+    {
+      name: "current ethers decodes the ENS custom error",
+      message: "execution reverted: ResolverNotFound(bytes)"
+    }
+  ];
+
+  for (const { name, message } of notRegistered) {
+    it(`detects an unregistered package: ${name}`, () => {
+      expect(isPackageNotRegisteredError(new Error(message))).to.equal(true);
+    });
+  }
+
+  // A registry problem must never be mistaken for an unpublished package, or
+  // publish would silently restart versioning from the manifest.
+  const realFailures: { name: string; message: string }[] = [
+    { name: "unrelated revert", message: "execution reverted: Unauthorized()" },
+    { name: "network failure", message: "could not detect network" },
+    { name: "bad response", message: "missing response for request" }
+  ];
+
+  for (const { name, message } of realFailures) {
+    it(`does not swallow a real failure: ${name}`, () => {
+      expect(isPackageNotRegisteredError(new Error(message))).to.equal(false);
+    });
+  }
+
+  it("tolerates a non-Error value", () => {
+    expect(isPackageNotRegisteredError(undefined)).to.equal(false);
+    expect(isPackageNotRegisteredError("execution reverted: ResolverNotFound(bytes)")).to.equal(true);
+  });
+});