Problems View does not display all Problems when they are reported to fast

[meisenbarth-work]

Bug Description:

The Problems View does not display all problems in the tree when problems are reported too fast. The counter at the panel is correct, but problems are missing,

The problems are added to an array, the adding of the problems is delayed, probably for performance reasons.

theia/packages/markers/src/browser/problem/problem-tree-model.ts

Line 82 in 6358553

this.markers.push({ node, markers });

The array is sorted, changing the order within the array.

theia/packages/markers/src/browser/problem/problem-tree-model.ts

Line 87 in 6358553

ProblemCompositeTreeNode.addChildren(this.root as MarkerRootNode, this.markers);

theia/packages/markers/src/browser/problem/problem-composite-tree-node.ts

Line 41 in 6358553

const sortedInsertChildren = insertChildren.sort(

The nodes are added, since the order changed, the node without errors and only warnings replaces the one with errors.

theia/packages/markers/src/browser/problem/problem-tree-model.ts

Line 89 in 6358553

for (const { node, markers } of this.markers) {

theia/packages/markers/src/browser/problem/problem-tree-model.ts

Line 92 in 6358553

this.setChildren(node, children);

theia/packages/core/src/browser/tree/tree.ts

Line 349 in 6358553

this.removeNode(parent);

It looks like the problem manager should provide all current problems for a specific file. Thus I assume it would be okay to remove other occurrences of the node from the markers array before adding a new entry? Otherwise a copy of the markers array could be sorted, but it looks like a waste to add nodes just to remove them in the next step again.

Steps to Reproduce:

1. Have a Problem Watcher that handles output
2. Have first warnings and than an error in the output, the error must be less than 50ms away from the last warning
3. The error is not displayed

Additional Information

• Operating System: Windows 10
• Theia Version: 1.45

[tsmaeder]

If I understand correctly, the problem already exists once we do

 this.markers.push({ node, markers }); 

If the line is invoked twice before the debounced doInsertNodesWithMarkers() method is invoked, we end up with two entries in this.markers for the same node.
Generally, I wonder why this model is so complicated: can't we just refresh the nodes and let the virtualized tree take care of performance issues?

@meisenbarth-work since you seem to have analyzed this problem thoroughly, would you fancy doing a PR to fix it?

[meisenbarth-work]

The debounce and array are from https://github.com/eclipse-theia/theia/pull/12408/files#diff-62bf371138c21435be92f9a1aedb041297aa9623f9218736eb930aff29dcd3f1 to fix a performance issue. Removing this (again) is probably not desired.

Regarding the PR, i would rather not, since I don't know how (technically and legally).

[msujew]

This will be fixed in #13841. I initially only wanted to fix #13835, but the root problem is the same - multiple nodes for the same file in the problem widget mess with the tree widget implementation. By replacing the array with a map, we circumvent the issue completely.

[tsmaeder]

@meisenbarth-work could you check your use case once the linked PR is merged, please?

[meisenbarth-work]

Sorry for the late reply, i can confirm the bug could not be reproduced anymore after applying the fix.
Thanks a lot.