Skip to content

Commit

Permalink
Improve errors in checkManifest (#2605)
Browse files Browse the repository at this point in the history
  • Loading branch information
ritave authored Jul 29, 2024
1 parent 0788b52 commit 7272f85
Show file tree
Hide file tree
Showing 14 changed files with 58 additions and 29 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion packages/snaps-utils/coverage.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
"branches": 99.73,
"functions": 98.9,
"lines": 99.43,
"statements": 96.31
"statements": 96.32
}
2 changes: 1 addition & 1 deletion packages/snaps-utils/src/manifest/manifest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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".',
]);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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".',
);
});
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { validate } from '@metamask/superstruct';

import { LocalizationFileStruct } from '../../localization';
import { getStructFailureMessage } from '../../structs';
import type { ValidatorMeta } from '../validator-types';

/**
Expand All @@ -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,
)}`,
);
}
}
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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".',
);
});
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { validate } from '@metamask/superstruct';

import { getStructFailureMessage } from '../../structs';
import { NpmSnapFileNames, NpmSnapPackageJsonStruct } from '../../types';
import type { ValidatorMeta } from '../validator-types';

Expand All @@ -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,
)}`,
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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".',
);
});
});
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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,
)}`,
);
}
}
Expand Down
5 changes: 5 additions & 0 deletions packages/snaps-utils/src/structs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,11 @@ export function getStructFailureMessage<Type, Schema>(
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,
Expand Down
21 changes: 12 additions & 9 deletions packages/snaps-webpack-plugin/src/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/snaps-webpack-plugin/src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
);
}

Expand Down
4 changes: 2 additions & 2 deletions scripts/install-chrome.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ set -u
set -o pipefail

# To get the latest version, see <https://www.ubuntuupdates.org/ppa/google_chrome?dist=stable>
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}"

Expand Down
10 changes: 5 additions & 5 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -10453,7 +10453,7 @@ __metadata:
tcp-port-used: ^1.0.2
bin:
chromedriver: bin/chromedriver
checksum: 755a866f19f64f4d2599844971566fe1c79659260f266ab53b8c943bfe0517fd9d1435e6c5dc7146c5266afcf7d11f748b5b981940cbcc5511856cba1996d292
checksum: f860fe6671aefd4c08c33e045340821e69cad8546790f4def69fedf2be0c8c27458d0a75f52a6394c06478f581fcad66e7e453f6d8dd352e6915f9a9e9b6e721
languageName: node
linkType: hard

Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 7272f85

Please sign in to comment.