Skip to content

Commit

Permalink
Breaking: Normalize error messages
Browse files Browse the repository at this point in the history 
Fix #1170
Fix #1133

Close #1208
  • Loading branch information
@alrra
alrra committed Aug 6, 2018
1 parent 2c5d2ad commit f50e933
Show file tree
Hide file tree
Showing 67 changed files with 693 additions and 543 deletions.
4 changes: 2 additions & 2 deletions packages/create-hint/src/templates/partial-event-code.hbs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ debug(`Validating hint {{name}}`);
/*
* This is where all the magic happens. Any errors found should be
* reported using the `context` object. E.g.:
* await context.report(resource, null, 'Your error message was here');
* await context.report(resource, null, 'Add error message here.');
*
* More information on how to develop a hint is available in:
* https://webhint.io/docs/contributor-guide/hints/
*/

if (Math.ceil(Math.random()) === 0) {
await context.report(resource, null, 'Your error message here');
await context.report(resource, null, 'Add error message here.');
}
12 changes: 6 additions & 6 deletions packages/create-hint/tests/fixtures/new-quotes.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,14 @@ export default class NewHint implements IHint {
/*
* This is where all the magic happens. Any errors found should be
* reported using the `context` object. E.g.:
* await context.report(resource, null, 'Your error message was here');
* await context.report(resource, null, 'Add error message here.');
*
* More information on how to develop a hint is available in:
* https://webhint.io/docs/contributor-guide/hints/
*/

if (Math.ceil(Math.random()) === 0) {
await context.report(resource, null, 'Your error message here');
await context.report(resource, null, 'Add error message here.');
}
};
const validateFetchEnd = async (fetchEnd: FetchEnd) => {
Expand All @@ -69,14 +69,14 @@ export default class NewHint implements IHint {
/*
* This is where all the magic happens. Any errors found should be
* reported using the `context` object. E.g.:
* await context.report(resource, null, 'Your error message was here');
* await context.report(resource, null, 'Add error message here.');
*
* More information on how to develop a hint is available in:
* https://webhint.io/docs/contributor-guide/hints/
*/

if (Math.ceil(Math.random()) === 0) {
await context.report(resource, null, 'Your error message here');
await context.report(resource, null, 'Add error message here.');
}
};
const validateFetchError = async (fetchError: FetchError) => {
Expand All @@ -89,14 +89,14 @@ export default class NewHint implements IHint {
/*
* This is where all the magic happens. Any errors found should be
* reported using the `context` object. E.g.:
* await context.report(resource, null, 'Your error message was here');
* await context.report(resource, null, 'Add error message here.');
*
* More information on how to develop a hint is available in:
* https://webhint.io/docs/contributor-guide/hints/
*/

if (Math.ceil(Math.random()) === 0) {
await context.report(resource, null, 'Your error message here');
await context.report(resource, null, 'Add error message here.');
}
};

Expand Down
12 changes: 6 additions & 6 deletions packages/create-hint/tests/fixtures/new.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@ const hint: IHintBuilder = {
/*
* This is where all the magic happens. Any errors found should be
* reported using the `context` object. E.g.:
* await context.report(resource, null, 'Your error message was here');
* await context.report(resource, null, 'Add error message here.');
*
* More information on how to develop a hint is available in:
* https://webhint.io/docs/contributor-guide/hints/
*/

if (Math.ceil(Math.random()) === 0) {
await context.report(resource, null, 'Your error message here');
await context.report(resource, null, 'Add error message here.');
}
};
const validateFetchEnd = async (fetchEnd: FetchEnd) => {
Expand All @@ -50,14 +50,14 @@ const hint: IHintBuilder = {
/*
* This is where all the magic happens. Any errors found should be
* reported using the `context` object. E.g.:
* await context.report(resource, null, 'Your error message was here');
* await context.report(resource, null, 'Add error message here.');
*
* More information on how to develop a hint is available in:
* https://webhint.io/docs/contributor-guide/hints/
*/

if (Math.ceil(Math.random()) === 0) {
await context.report(resource, null, 'Your error message here');
await context.report(resource, null, 'Add error message here.');
}
};
const validateFetchError = async (fetchError: FetchError) => {
Expand All @@ -70,14 +70,14 @@ const hint: IHintBuilder = {
/*
* This is where all the magic happens. Any errors found should be
* reported using the `context` object. E.g.:
* await context.report(resource, null, 'Your error message was here');
* await context.report(resource, null, 'Add error message here.');
*
* More information on how to develop a hint is available in:
* https://webhint.io/docs/contributor-guide/hints/
*/

if (Math.ceil(Math.random()) === 0) {
await context.report(resource, null, 'Your error message here');
await context.report(resource, null, 'Add error message here.');
}
};

Expand Down
12 changes: 6 additions & 6 deletions packages/create-parser/tests/fixtures/new-quotes.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,14 @@ export default class NewHint implements IHint {
/*
* This is where all the magic happens. Any errors found should be
* reported using the `context` object. E.g.:
* await context.report(resource, null, 'Your error message was here');
* await context.report(resource, null, 'Add error message here.');
*
* More information on how to develop a hint is available in:
* https://webhint.io/docs/contributor-guide/hints/
*/

if (Math.ceil(Math.random()) === 0) {
await context.report(resource, null, 'Your error message here');
await context.report(resource, null, 'Add error message here.');
}
};
const validateFetchEnd = async (fetchEnd: FetchEnd) => {
Expand All @@ -69,14 +69,14 @@ export default class NewHint implements IHint {
/*
* This is where all the magic happens. Any errors found should be
* reported using the `context` object. E.g.:
* await context.report(resource, null, 'Your error message was here');
* await context.report(resource, null, 'Add error message here.');
*
* More information on how to develop a hint is available in:
* https://webhint.io/docs/contributor-guide/hints/
*/

if (Math.ceil(Math.random()) === 0) {
await context.report(resource, null, 'Your error message here');
await context.report(resource, null, 'Add error message here.');
}
};
const validateFetchError = async (fetchError: FetchError) => {
Expand All @@ -89,14 +89,14 @@ export default class NewHint implements IHint {
/*
* This is where all the magic happens. Any errors found should be
* reported using the `context` object. E.g.:
* await context.report(resource, null, 'Your error message was here');
* await context.report(resource, null, 'Add error message here.');
*
* More information on how to develop a hint is available in:
* https://webhint.io/docs/contributor-guide/hints/
*/

if (Math.ceil(Math.random()) === 0) {
await context.report(resource, null, 'Your error message here');
await context.report(resource, null, 'Add error message here.');
}
};

Expand Down
12 changes: 6 additions & 6 deletions packages/create-parser/tests/fixtures/new.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@ const hint: IHintBuilder = {
/*
* This is where all the magic happens. Any errors found should be
* reported using the `context` object. E.g.:
* await context.report(resource, null, 'Your error message was here');
* await context.report(resource, null, 'Add error message here.');
*
* More information on how to develop a hint is available in:
* https://webhint.io/docs/contributor-guide/hints/
*/

if (Math.ceil(Math.random()) === 0) {
await context.report(resource, null, 'Your error message here');
await context.report(resource, null, 'Add error message here.');
}
};
const validateFetchEnd = async (fetchEnd: FetchEnd) => {
Expand All @@ -50,14 +50,14 @@ const hint: IHintBuilder = {
/*
* This is where all the magic happens. Any errors found should be
* reported using the `context` object. E.g.:
* await context.report(resource, null, 'Your error message was here');
* await context.report(resource, null, 'Add error message here.');
*
* More information on how to develop a hint is available in:
* https://webhint.io/docs/contributor-guide/hints/
*/

if (Math.ceil(Math.random()) === 0) {
await context.report(resource, null, 'Your error message here');
await context.report(resource, null, 'Add error message here.');
}
};
const validateFetchError = async (fetchError: FetchError) => {
Expand All @@ -70,14 +70,14 @@ const hint: IHintBuilder = {
/*
* This is where all the magic happens. Any errors found should be
* reported using the `context` object. E.g.:
* await context.report(resource, null, 'Your error message was here');
* await context.report(resource, null, 'Add error message here.');
*
* More information on how to develop a hint is available in:
* https://webhint.io/docs/contributor-guide/hints/
*/

if (Math.ceil(Math.random()) === 0) {
await context.report(resource, null, 'Your error message here');
await context.report(resource, null, 'Add error message here.');
}
};

Expand Down
10 changes: 8 additions & 2 deletions packages/hint-amp-validator/src/hint.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,10 @@ export default class AmpValidatorHint implements IHint {
return;
}

// events has to be an array in order to work with the local connector.
/*
* `events` has to be an array in order
* to work with the local connector.
*/
events.push(fetchEnd);
validPromise = amphtmlValidator.getInstance();
};
Expand All @@ -70,7 +73,10 @@ export default class AmpValidatorHint implements IHint {
message += ` (${error.specUrl})`;
}

// We ignore errors that are not 'ERROR' if user has configured the hint like that
/*
* We ignore errors that are not 'ERROR'
* if user has configured the hint like that.
*/
if (errorsOnly && error.severity !== 'ERROR') {
debug(`AMP error doesn't meet threshold for reporting`);
} else {
Expand Down
31 changes: 13 additions & 18 deletions packages/hint-apple-touch-icons/src/hint.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,14 @@ export default class AppleTouchIconsHint implements IHint {
*/

if (!appleTouchIconHref) {
await context.report(resource, appleTouchIcon, `'apple-touch-icon' should have non-empty 'href' attribute`);
await context.report(resource, appleTouchIcon, `'apple-touch-icon' link element should have non-empty 'href' attribute.`);

return;
}

// - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

/*
* - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
* The following checks don't make sense for non-HTTP(S).
*/

Expand Down Expand Up @@ -120,15 +121,15 @@ export default class AppleTouchIconsHint implements IHint {
networkData = await context.fetchContent(appleTouchIconURL);
} catch (e) {
debug(`Failed to fetch the ${appleTouchIconHref} file`);
await context.report(resource, appleTouchIcon, `'${appleTouchIconHref}' file request failed`);
await context.report(resource, appleTouchIcon, `'${appleTouchIconHref}' could not be fetched (request failed).`);

return;
}

const response = networkData.response;

if (response.statusCode !== 200) {
await context.report(resource, appleTouchIcon, `'${appleTouchIconHref}' could not be fetched (status code: ${response.statusCode})`);
await context.report(resource, appleTouchIcon, `'${appleTouchIconHref}' could not be fetched (status code: ${response.statusCode}).`);

return;
}
Expand All @@ -154,7 +155,7 @@ export default class AppleTouchIconsHint implements IHint {
image = getImageData(response.body.rawContent);
} catch (e) {
if (e instanceof TypeError) {
await context.report(resource, appleTouchIcon, `'${appleTouchIconHref}' is not a valid PNG`);
await context.report(resource, appleTouchIcon, `'${appleTouchIconHref}' should be a valid PNG image.`);
} else {
debug(`'getImageData' failed for '${appleTouchIconURL}'`);
}
Expand All @@ -165,13 +166,13 @@ export default class AppleTouchIconsHint implements IHint {
// Check if the image is a PNG.

if (image.type !== 'png') {
await context.report(resource, appleTouchIcon, `'${appleTouchIconHref}' is not a PNG`);
await context.report(resource, appleTouchIcon, `'${appleTouchIconHref}' should be a PNG image.`);
}

// Check if the image is 180x180px.

if (image.width !== 180 || image.height !== 180) {
await context.report(resource, appleTouchIcon, `'${appleTouchIconHref}' is not 180x180px`);
await context.report(resource, appleTouchIcon, `'${appleTouchIconHref}' should be 180x180px.`);
}

// TODO: Check if the image has some kind of transparency.
Expand Down Expand Up @@ -218,31 +219,28 @@ export default class AppleTouchIconsHint implements IHint {
const appleTouchIcons: Array<IAsyncHTMLElement> = getAppleTouchIcons(await pageDOM.querySelectorAll('link'));

if (appleTouchIcons.length === 0) {
await context.report(resource, null, `No 'apple-touch-icon' was specified`);
await context.report(resource, null, `'apple-touch-icon' link element was not specified.`);

return;
}

/*
* - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
* Choose the icon that will most likely
* pass most of the following tests.
*/

const appleTouchIcon: IAsyncHTMLElement = chooseBestIcon(appleTouchIcons);

/*
* - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
* Check if `rel='apple-touch-icon'`.
* See `getAppleTouchIcons` function for more details.
*/

if (normalizeString(appleTouchIcon.getAttribute('rel')) !== 'apple-touch-icon') {
await context.report(resource, appleTouchIcon, `'rel' attribute value should be 'apple-touch-icon'`);
await context.report(resource, appleTouchIcon, `'apple-touch-icon' link element should have 'rel="apple-touch-icon".`);
}

/*
* - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
* Since we are recommending one icon, the `sizes` attribute
* is not needed. Also, pre-4.2 versions of iOS ignore the
* `sizes` attribute.
Expand All @@ -252,38 +250,35 @@ export default class AppleTouchIconsHint implements IHint {
*/

if (appleTouchIcon.getAttribute('sizes')) {
await context.report(resource, appleTouchIcon, `'sizes' attribute is not needed`);
await context.report(resource, appleTouchIcon, `'apple-touch-icon' link element should not have 'sizes' attribute.`);
}

/*
* - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
* Check if the `apple-touch-icon` exists, is the right
* image format, the right size, etc.
*/

await checkImage(appleTouchIcon, resource);

/*
* - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
* Check if the `apple-touch-icon` is included in the `<body>`.
*/

const bodyAppleTouchIcons: Array<IAsyncHTMLElement> = getAppleTouchIcons(await pageDOM.querySelectorAll('body link'));

for (const icon of bodyAppleTouchIcons) {
if (icon.isSame(appleTouchIcon)) {
await context.report(resource, appleTouchIcon, `'apple-touch-icon' should be specified in the '<head>'`);
await context.report(resource, appleTouchIcon, `'apple-touch-icon' link element should be specified in the '<head>'.`);
}
}

/*
* - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
* All other `apple-touch-icon`s should not be included.
*/

for (const icon of appleTouchIcons) {
if (!icon.isSame(appleTouchIcon)) {
await context.report(resource, icon, `A 'apple-touch-icon' was already specified`);
await context.report(resource, icon, `'apple-touch-icon' link element is not needed as one was already specified.`);
}
}
};
Expand Down
Loading

0 comments on commit f50e933

Please sign in to comment.