Skip to content

frontend: Replace use of errors.Cause with errors.Is.

Warren Giffordrequested to merge
vg/errors-is-not-cause into main

Created by: varungandhi-src

errors.UnwrapAll (called internally by errors.Cause) can return a more 'bare' error than the original error created by errors.New. This means that errors.Cause would have 0 stack frames; comparing that to a result from errors.New (with 1 stack frame) would fail. Thanks to this counter-intuitive behavior, switching over the result of errors.Cause is (ahem) error-prone; it MUST NOT be compared directly with the result of errors.New. errors.Is does not suffer from this problem.

Fixes issue #33697 (closed); we had code paths for making sure that we showed the un-highlighted file on HSS timeouts, but that code path was not triggered because we weren't checking the error's identity properly.

I think it is better to be paranoid here given that a regression in this functionality adversely affects customers in a meaningful way. That is why I've added a test here for seemingly trivial functionality. The test should hopefully prevent someone from carelessly "cleaning up" the if-else chain to use errors.Cause + switch.

(Slack discussion)

Test plan

Added automated test.

Merge request reports