Skip to content

compute+insights: add compute stream error handling

Warren Gifford requested to merge compute/fix-stream-error-channel into main

Created by: leonore

closes #34230 (closed), again

This contains work from #34650 which was reverted after a bug was found. The bug was that we were seemingly encountering a deadlock because of the buffered error channel. Not sure how this wasn't found during testing for #34650, probably a lack of thorough checking on happy paths.

Test plan

See #34650 for initial test plan.

This was tested against compute integration tests at #35539.

Test plan for the bug

Make a just-in-time capture group insight on your local instance by specifying to run the search on 1 repository only. https://sourcegraph.test:3443/insights/create/capture-group?dashboardId=all repo: github.com/sourcegraph/sourcegraph example query: patterntype:regexp TODO([\w])

On main this would currently be timing out. The faulty commit is 89e066f. If you checkout its parent, fbdeae5, you can reproduce the behaviour where this works:

git checkout fbdeae5

On main with some logging we see we get blocked somewhere.

## blocking
[enterprise-frontend] INFO [{O 1} {k 3}]
[enterprise-frontend] INFO after timeGroupElement
[enterprise-frontend] INFO before decoder
[enterprise-frontend] INFO after decoder
[enterprise-frontend] INFO after streaming
[enterprise-frontend] INFO [{k 12} {O 1}]
[enterprise-frontend] INFO after timeGroupElement
[enterprise-frontend] INFO before decoder
[enterprise-frontend] INFO after decoder

## no blocking                                                                                    
[enterprise-frontend] INFO before decoder                                                                                                 
[enterprise-frontend] INFO after decoder                                                                                                  
[enterprise-frontend] INFO after streaming                                                                                                
[enterprise-frontend] INFO [{k 11} {z 1}]                                                                                                 
[enterprise-frontend] INFO after timeGroupElement                                                                                         
[enterprise-frontend] INFO makeTimeSeries                                                                                                 
[enterprise-frontend] INFO after calculated

Applying the fix implemented in this PR, we go back to normal behaviour and see a live preview.

Merge request reports