From 7272f85203a1c9e402184bbbd17f6420d3f0a09b Mon Sep 17 00:00:00 2001 From: Olaf Tomalka Date: Mon, 29 Jul 2024 12:18:06 +0200 Subject: [PATCH] Improve errors in checkManifest (#2605) --- package.json | 2 +- packages/snaps-utils/coverage.json | 2 +- .../snaps-utils/src/manifest/manifest.test.ts | 2 +- .../validators/is-localization-file.test.ts | 2 +- .../validators/is-localization-file.ts | 15 ++++++++++--- .../validators/is-package-json.test.ts | 2 +- .../manifest/validators/is-package-json.ts | 9 +++++++- .../validators/is-snap-manifest.test.ts | 2 +- .../manifest/validators/is-snap-manifest.ts | 7 ++++++- packages/snaps-utils/src/structs.ts | 5 +++++ .../snaps-webpack-plugin/src/plugin.test.ts | 21 +++++++++++-------- packages/snaps-webpack-plugin/src/plugin.ts | 4 ++-- scripts/install-chrome.sh | 4 ++-- yarn.lock | 10 ++++----- 14 files changed, 58 insertions(+), 29 deletions(-) diff --git a/package.json b/package.json index 2e9eb0a028..d0431ad32b 100644 --- a/package.json +++ b/package.json @@ -82,7 +82,7 @@ "@types/node": "18.14.2", "@typescript-eslint/eslint-plugin": "^5.42.1", "@typescript-eslint/parser": "^5.42.1", - "chromedriver": "^125.0.1", + "chromedriver": "^127.0.0", "depcheck": "^1.4.7", "eslint": "^8.27.0", "eslint-config-prettier": "^8.5.0", diff --git a/packages/snaps-utils/coverage.json b/packages/snaps-utils/coverage.json index 0f4e8a643a..fab111efc3 100644 --- a/packages/snaps-utils/coverage.json +++ b/packages/snaps-utils/coverage.json @@ -2,5 +2,5 @@ "branches": 99.73, "functions": 98.9, "lines": 99.43, - "statements": 96.31 + "statements": 96.32 } diff --git a/packages/snaps-utils/src/manifest/manifest.test.ts b/packages/snaps-utils/src/manifest/manifest.test.ts index a66d14cd52..c8486fa2bc 100644 --- a/packages/snaps-utils/src/manifest/manifest.test.ts +++ b/packages/snaps-utils/src/manifest/manifest.test.ts @@ -228,7 +228,7 @@ describe('checkManifest', () => { const { reports } = await checkManifest(BASE_PATH); expect(reports.map(({ message }) => message)).toStrictEqual([ - 'Failed to validate localization file "/snap/locales/en.json": At path: messages -- Expected an object, but received: "foo".', + 'Failed to validate localization file "/snap/locales/en.json": At path: messages — Expected a value of type record, but received: "foo".', ]); }); diff --git a/packages/snaps-utils/src/manifest/validators/is-localization-file.test.ts b/packages/snaps-utils/src/manifest/validators/is-localization-file.test.ts index 7e8708244e..13cbc3237e 100644 --- a/packages/snaps-utils/src/manifest/validators/is-localization-file.test.ts +++ b/packages/snaps-utils/src/manifest/validators/is-localization-file.test.ts @@ -52,7 +52,7 @@ describe('isLocalizationFile', () => { ); expect(report).toHaveBeenCalledWith( - 'Failed to validate localization file "/foo": At path: messages -- Expected an object, but received: "foo".', + 'Failed to validate localization file "/foo": At path: messages — Expected a value of type record, but received: "foo".', ); }); }); diff --git a/packages/snaps-utils/src/manifest/validators/is-localization-file.ts b/packages/snaps-utils/src/manifest/validators/is-localization-file.ts index e0dfe80529..4b21947dd3 100644 --- a/packages/snaps-utils/src/manifest/validators/is-localization-file.ts +++ b/packages/snaps-utils/src/manifest/validators/is-localization-file.ts @@ -1,6 +1,7 @@ import { validate } from '@metamask/superstruct'; import { LocalizationFileStruct } from '../../localization'; +import { getStructFailureMessage } from '../../structs'; import type { ValidatorMeta } from '../validator-types'; /** @@ -13,9 +14,17 @@ export const isLocalizationFile: ValidatorMeta = { const [error] = validate(file.result, LocalizationFileStruct); if (error) { - context.report( - `Failed to validate localization file "${file.path}": ${error.message}.`, - ); + for (const failure of error.failures()) { + context.report( + `Failed to validate localization file "${ + file.path + }": ${getStructFailureMessage( + LocalizationFileStruct, + failure, + false, + )}`, + ); + } } } }, diff --git a/packages/snaps-utils/src/manifest/validators/is-package-json.test.ts b/packages/snaps-utils/src/manifest/validators/is-package-json.test.ts index 8c226bec39..1f3142bb63 100644 --- a/packages/snaps-utils/src/manifest/validators/is-package-json.test.ts +++ b/packages/snaps-utils/src/manifest/validators/is-package-json.test.ts @@ -64,7 +64,7 @@ describe('isPackageJson', () => { ); expect(report).toHaveBeenCalledWith( - '"package.json" is invalid: Expected SemVer version, got "foo"', + '"package.json" is invalid: At path: version — Expected SemVer version, got "foo".', ); }); }); diff --git a/packages/snaps-utils/src/manifest/validators/is-package-json.ts b/packages/snaps-utils/src/manifest/validators/is-package-json.ts index e48b927bd5..3d3ea7dd78 100644 --- a/packages/snaps-utils/src/manifest/validators/is-package-json.ts +++ b/packages/snaps-utils/src/manifest/validators/is-package-json.ts @@ -1,5 +1,6 @@ import { validate } from '@metamask/superstruct'; +import { getStructFailureMessage } from '../../structs'; import { NpmSnapFileNames, NpmSnapPackageJsonStruct } from '../../types'; import type { ValidatorMeta } from '../validator-types'; @@ -19,7 +20,13 @@ export const isPackageJson: ValidatorMeta = { if (error) { for (const failure of error.failures()) { context.report( - `"${NpmSnapFileNames.PackageJson}" is invalid: ${failure.message}`, + `"${ + NpmSnapFileNames.PackageJson + }" is invalid: ${getStructFailureMessage( + NpmSnapPackageJsonStruct, + failure, + false, + )}`, ); } } diff --git a/packages/snaps-utils/src/manifest/validators/is-snap-manifest.test.ts b/packages/snaps-utils/src/manifest/validators/is-snap-manifest.test.ts index 443616dfb8..1a71b13b42 100644 --- a/packages/snaps-utils/src/manifest/validators/is-snap-manifest.test.ts +++ b/packages/snaps-utils/src/manifest/validators/is-snap-manifest.test.ts @@ -42,7 +42,7 @@ describe('isSnapManifest', () => { ); expect(report).toHaveBeenCalledWith( - '"snap.manifest.json" is invalid: Expected an object, but received: "foo"', + '"snap.manifest.json" is invalid: Expected a value of type object, but received: "foo".', ); }); }); diff --git a/packages/snaps-utils/src/manifest/validators/is-snap-manifest.ts b/packages/snaps-utils/src/manifest/validators/is-snap-manifest.ts index 2b869dbf04..411344e55e 100644 --- a/packages/snaps-utils/src/manifest/validators/is-snap-manifest.ts +++ b/packages/snaps-utils/src/manifest/validators/is-snap-manifest.ts @@ -1,5 +1,6 @@ import { validate } from '@metamask/superstruct'; +import { getStructFailureMessage } from '../../structs'; import { NpmSnapFileNames } from '../../types'; import { SnapManifestStruct } from '../validation'; import type { ValidatorMeta } from '../validator-types'; @@ -17,7 +18,11 @@ export const isSnapManifest: ValidatorMeta = { if (error) { for (const failure of error.failures()) { context.report( - `"${NpmSnapFileNames.Manifest}" is invalid: ${failure.message}`, + `"${NpmSnapFileNames.Manifest}" is invalid: ${getStructFailureMessage( + SnapManifestStruct, + failure, + false, + )}`, ); } } diff --git a/packages/snaps-utils/src/structs.ts b/packages/snaps-utils/src/structs.ts index f6e548712d..1881c19b57 100644 --- a/packages/snaps-utils/src/structs.ts +++ b/packages/snaps-utils/src/structs.ts @@ -318,6 +318,11 @@ export function getStructFailureMessage( return `${prefix}${message}.`; } + // Refinements we built ourselves have nice error messages + if (failure.refinement !== undefined) { + return `${prefix}${failure.message}.`; + } + return `${prefix}Expected a value of type ${color( failure.type, green, diff --git a/packages/snaps-webpack-plugin/src/plugin.test.ts b/packages/snaps-webpack-plugin/src/plugin.test.ts index 71452fa5a2..d934c74900 100644 --- a/packages/snaps-webpack-plugin/src/plugin.test.ts +++ b/packages/snaps-webpack-plugin/src/plugin.test.ts @@ -252,15 +252,18 @@ describe('SnapsWebpackPlugin', () => { ], }); - await expect( - bundle({ - options: { - eval: false, - manifestPath: '/snap.manifest.json', - writeManifest: false, - }, - }), - ).rejects.toThrow('Manifest Error: The manifest is invalid.\nfoo\nbar'); + const { stats } = await bundle({ + options: { + eval: false, + manifestPath: '/snap.manifest.json', + writeManifest: false, + }, + }); + + // eslint-disable-next-line jest/prefer-strict-equal + expect(stats.compilation.errors.map((error) => error.message)).toEqual( + expect.arrayContaining(['foo', 'bar']), + ); }); it('logs manifest warnings', async () => { diff --git a/packages/snaps-webpack-plugin/src/plugin.ts b/packages/snaps-webpack-plugin/src/plugin.ts index bafdec8787..6fadf5c679 100644 --- a/packages/snaps-webpack-plugin/src/plugin.ts +++ b/packages/snaps-webpack-plugin/src/plugin.ts @@ -164,8 +164,8 @@ export default class SnapsWebpackPlugin { .map((report) => report.message); if (errors.length > 0) { - throw new Error( - `Manifest Error: The manifest is invalid.\n${errors.join('\n')}`, + compilation.errors.push( + ...errors.map((error) => new WebpackError(error)), ); } diff --git a/scripts/install-chrome.sh b/scripts/install-chrome.sh index ef08e28de7..5ebf6a8a52 100755 --- a/scripts/install-chrome.sh +++ b/scripts/install-chrome.sh @@ -5,12 +5,12 @@ set -u set -o pipefail # To get the latest version, see -CHROME_VERSION='125.0.6422.76-1' +CHROME_VERSION='127.0.6533.72-1' CHROME_BINARY="google-chrome-stable_${CHROME_VERSION}_amd64.deb" CHROME_BINARY_URL="https://dl.google.com/linux/chrome/deb/pool/main/g/google-chrome-stable/${CHROME_BINARY}" # To retrieve this checksum, run the `wget` and `shasum` commands below -CHROME_BINARY_SHA512SUM='0c221bca2bfaf198018f8d1649da2ae3120e3a3e27dcf9c16170a6b05302728d28caf8af172bdd6e34b56d3b6cc7769b4a17def250c92a569871565d167dc866' +CHROME_BINARY_SHA512SUM='54097b33fbe8bf485273f25446b5da36fdae1b4a3d1722f3b245b63f7337f5acdae1788000ca7b6817e8197c675785f2c94e487426126c734e7ca725affb7f2a' wget -O "${CHROME_BINARY}" -t 5 "${CHROME_BINARY_URL}" diff --git a/yarn.lock b/yarn.lock index db6fdce732..de1c213872 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10440,9 +10440,9 @@ __metadata: languageName: node linkType: hard -"chromedriver@npm:^125.0.1": - version: 125.0.1 - resolution: "chromedriver@npm:125.0.1" +"chromedriver@npm:^127.0.0": + version: 127.0.0 + resolution: "chromedriver@npm:127.0.0" dependencies: "@testim/chrome-version": ^1.1.4 axios: ^1.6.7 @@ -10453,7 +10453,7 @@ __metadata: tcp-port-used: ^1.0.2 bin: chromedriver: bin/chromedriver - checksum: 755a866f19f64f4d2599844971566fe1c79659260f266ab53b8c943bfe0517fd9d1435e6c5dc7146c5266afcf7d11f748b5b981940cbcc5511856cba1996d292 + checksum: f860fe6671aefd4c08c33e045340821e69cad8546790f4def69fedf2be0c8c27458d0a75f52a6394c06478f581fcad66e7e453f6d8dd352e6915f9a9e9b6e721 languageName: node linkType: hard @@ -20306,7 +20306,7 @@ __metadata: "@types/node": 18.14.2 "@typescript-eslint/eslint-plugin": ^5.42.1 "@typescript-eslint/parser": ^5.42.1 - chromedriver: ^125.0.1 + chromedriver: ^127.0.0 depcheck: ^1.4.7 eslint: ^8.27.0 eslint-config-prettier: ^8.5.0