search: guard against panic for missing repo broadcast on nil stream
Created by: rvantonder
The function MissingRepoRevStatus()
takes a stream and sends status messages if there are missing repos. Some time ago, we maintained the implicit invariant that this stream
is non-nil. In a previous commit, I broke this invariant where it became possible to call this function with a nil stream
on this line: https://github.com/sourcegraph/sourcegraph/commit/95dba9b3b064dbf1031734ca5aceda0058856abc#diff-81779cda5e11347c5a456ef4c0c0b462ad0cbab0d2a86a261337ff4be277b7cfR626
@coury-clark this is the issue that likely caused your trouble using the GQL API in thread
I've audited the calls for other MissingRepoRevStatus
constructors and I'm quite sure they're safe, but to make doubly sure I've put the nil
check inside this function, rather than the one caller that broke the invariant. It does make me want to put a panic("broken invariant: non-nil stream")
in this function here, but the code calling into is bound to clear up soon. Right now we're tiptoeing around the value of stream
in our backend, whether it is nil
or not, that's holding for now, and it'll become easier to reason over when we concentrate the logic for stream is nil?
in one place.
I could write a test for MissingRepoRevStatus
that checks whether this returns a noop function, but it's honestly low value. Open to being persuaded.
CI currently broken, please ignore and stamp.