Skip to content

Fix double-wrapped no-network issue in createKnowledgeFromData#143

Open
marcusosterberg wants to merge 1 commit into
mainfrom
fix/no-network-issue-double-wrapped
Open

Fix double-wrapped no-network issue in createKnowledgeFromData#143
marcusosterberg wants to merge 1 commit into
mainfrom
fix/no-network-issue-double-wrapped

Conversation

@marcusosterberg
Copy link
Copy Markdown
Contributor

Summary

Removes an extra 'no-network' wrapper object from the issue emitted
by createKnowledgeFromData when the target page could not be loaded.
The issue now has the same shape as in plugin-accessibility-statement
and the rest of lib/harAnalyzer.js.

Fixes #.

Before

knowledgeData.issues = {
  'no-network': {
    'no-network': {            // extra wrapper
      rule: 'no-network',
      severity: 'warning',
      subIssues: [ ... ]
    }
  }
}

After

knowledgeData.issues = {
  'no-network': {
    rule: 'no-network',
    severity: 'warning',
    subIssues: [ ... ]
  }
}

Verification

Tested by running python default.py -t 2 -u <site> -r -o t2.json
against a site that triggers browsertime's UrlLoadError. Before
the fix, this crashes downstream (Cannot read properties of undefined (reading 'length') in plugin-webperf-core's pug template,
then KeyError: 'severity' in webperf_core). After the fix the test
completes and reports no-network as a warning issue.

When the target page cannot be loaded (e.g. browsertime UrlLoadError),
createKnowledgeFromData wrapped the no-network issue in an extra
'no-network' key, producing knowledgeData.issues of the shape:

    { 'no-network': { 'no-network': { rule, severity, subIssues, ... } } }

instead of the expected:

    { 'no-network': { rule, severity, subIssues, ... } }

This caused downstream consumers (plugin-webperf-core's pug template
and webperf_core's calculate_rating) to fail because severity and
subIssues were one level too deep.

Aligns the shape with plugin-accessibility-statement and the rest of
the no-network handling in this file.

Fixes #142
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant