Skip to content

core(errors-in-console): ignore BLOCKED_BY_CLIENT.Inspector errors #11901

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
devtools-bot merged 4 commits into from
Jan 12, 2021
Merged

core(errors-in-console): ignore BLOCKED_BY_CLIENT.Inspector errors #11901

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ee79c62
core(errors-in-console): ignore BLOCKED_BY_CLIENT.Inspector errors
paulirish Dec 29, 2020
f8e4294
Update lighthouse-core/test/audits/errors-in-console-test.js
paulirish Dec 29, 2020
2ad2cd2
move test to a new defaultOptions group
paulirish Jan 12, 2021
c9662ce
add small comment
paulirish Jan 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lighthouse-core/audits/errors-in-console.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ class ErrorLogs extends Audit {

/** @return {AuditOptions} */
static defaultOptions() {
return {};
// Any failed network requests with error messsage aren't actionable
return {ignoredPatterns: ['ERR_BLOCKED_BY_CLIENT.Inspector']};
Copy link
Collaborator

@patrickhulce patrickhulce Jan 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My initial reaction was .Inspector might be too narrow since I didn't remember it always having that suffix, but I couldn't find reports of it without it, so I might be making that up. Calling it out in case you had the same initial thought and were on the fence for another nudge :)

Copy link
Member Author

@paulirish paulirish Jan 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a bunch of other blockedReasons that are fairly legit, so i think preserving those results seems good. https://chromedevtools.github.io/devtools-protocol/tot/Network/#type-BlockedReason

Copy link
Collaborator

@patrickhulce patrickhulce Jan 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya for sure, but those would be caught by {ignoredPatterns: ['ERR_BLOCKED_BY_CLIENT']}? I was questioning if the .Inspector part needs to be here.

}


/**
* @template {{description: string | undefined}} T
* @param {Array<T>} items
Expand Down
17 changes: 17 additions & 0 deletions lighthouse-core/test/audits/errors-in-console-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,4 +224,21 @@ describe('ConsoleMessages error logs audit', () => {
expect(result.details.items).toHaveLength(0);
});
});

describe('defaultOptions', () => {
// See https://github.com/GoogleChrome/lighthouse/issues/10198
it('filters out blocked_by_client.inspector messages by default', () => {
const auditResult = ErrorLogsAudit.audit({
ConsoleMessages: [{
'source': 'exception',
'level': 'error',
'timestamp': 1506535813608.003,
'url': 'https://www.facebook.com/tr/',
'text': 'Failed to load resource: net::ERR_BLOCKED_BY_CLIENT.Inspector',
}],
}, {options: ErrorLogsAudit.defaultOptions()});
assert.equal(auditResult.score, 1);
assert.equal(auditResult.details.items.length, 0);
});
});
});