Skip to content

fix: comment controller can be disposed regardless of API failures

Tomas Vik (OOO back on 2024-07-02) requested to merge 342-dispose-comment-controller into main

By sheer luck, I found an edge case that causes a small bug in the MR review process. The issue manifests itself as multiple comment controllers for the diff (making the user select which to use for making comments).

The issue is caused by the distance between creating a comment controller (VS Code object responsible for showing and creating comments) and the line of code that registers this controller to be disposed (VS Code term for releasing resource when it's not needed). In between these two actions we were making API call. If this API call failed, we never disposed the controller causing multiple controllers to be available for a file.

The video shows how API failure causes two controllers to be present after refresh:

double-comment-controller

Order before this MR:

  1. create comment controller
  2. make an API call to get MR discussions
  3. register the controller for disposal (the disposal happens when we click refresh)

Order after this MR:

  1. create comment controller
  2. register the controller for disposal
  3. make an API call to get MR discussions

Related to #342 (closed)

Merge request reports