Skip to content
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.

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

Check for null child node/Ids #224

Merged
merged 2 commits into from
Feb 27, 2023
Merged

Conversation

philipbalvanz-wf
Copy link
Contributor

I've been seeing the following error intermittently when running puppeteer-dart, which is causing test run failures.

Null check operator used on a null value
package:puppeteer/src/page/accessibility.dart 548:53  _AXNode.createTree
package:puppeteer/src/page/accessibility.dart 69:31   Accessibility.snapshot

I was able to recreate this scenario with the following code:

class AXNodeId {
  final String value;
  AXNodeId(this.value);

  factory AXNodeId.fromJson(String value) => AXNodeId(value);
}

class AXNodeData {
  final List<AXNodeId>? childIds;

  AXNodeData({this.childIds});

  factory AXNodeData.fromJson(Map<String, dynamic> json) {
    return AXNodeData(
        childIds: json.containsKey('childIds')
            ? (json['childIds'] as List)
            .map((e) => AXNodeId.fromJson(e as String))
            .toList()
            : null
    );
  }
}

class _AXNode {
  final AXNodeData _payload;
  final _children = <_AXNode>[];
  _AXNode(this._payload);
}

void main() {
  final nodeData = AXNodeData.fromJson({'childIds':['123']});
  var nodeById = <String, _AXNode>{};
  final axNode = _AXNode(nodeData);

  for(var childId in axNode._payload.childIds!){
    axNode._children.add(nodeById[childId.value]!);
  }
}

The issue appears to be related to the use of the null assertion operator (!) on a value that can in fact be null in this case. From what I can tell nodeById does not contain the childId.value in the map, thus resulting in null being returned. This appears to indicate that the child node is missing? from the FullAXTree(removed during processing?) or we've been given a bad id. Let me know if the FullAXTree missing a node is cause for concern.

My proposal is to explicitly check for null after retrieving the value, prior to adding it to the list of children. I also updated the node._payload.childIds! to prevent the nesting from getting any deeper.

Overall it feels like the null assertion operator ! should be used sparingly, as the analyzer simply shuts off, even if you didn't check for null first.

@xvrh for your review

@xvrh xvrh merged commit 1d7055e into xvrh:master Feb 27, 2023
@xvrh
Copy link
Owner

xvrh commented Feb 27, 2023

Thanks for the contribution!

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.

2 participants