search: fix regression where search result highlighting is not bailed…

… out after 3s (#4369)

* search: fix regression where search result highlighting is not bailed out after 3s

Prior to this change there was a major regression in which we would disable the
syntax highlighting timeout outright. This is despite the fact that this should
only ever be done when the user has communicated an intent to wait longer for
search results.

This regression meant that when a search turned up files that the syntax highlighter could
not highlight quickly, we would just wait for them until it finished. In one customer
environment I observed search results taking 30s+ to load due to this because it had the
added negative consequence of us not canceling those poorly performing requests which led to
syntect_server trying to perform tons of CPU-intensive requests which harmed other requests
it was handling.

After this change, we are properly bailing out once again after the syntax highlighting
timeout.

This means that in this bad case you are waiting 3s then getting plaintext results instead of
waiting 30s+ for highlighting.

Fixes #4268

Test plan: Manually tested using #4364 & added a regression test for one unit

CHANGELOG.md

@@ -44,6 +44,7 @@ All notable changes to Sourcegraph are documented in this file.
 - Fixed an issue with `sourcegraph/server` Docker deployments where syntax highlighting could produce `server closed idle connection` errors. [#4269](https://github.com/sourcegraph/sourcegraph/issues/4269)
 - Repositories containing submodules not on Sourcegraph will now load without error [#2947](https://github.com/sourcegraph/sourcegraph/issues/2947)
 - HTTP metrics in Prometheus/Grafana now distinguish between different types of GraphQL requests.
+- Fixed an issue where syntax highlighting taking too long would result in errors or wait long amounts of time without properly falling back to plaintext rendering after a few seconds. [#4267](https://github.com/sourcegraph/sourcegraph/issues/4267) [#4268](https://github.com/sourcegraph/sourcegraph/issues/4268)
 
 ## 3.4.2
 

shared/src/components/CodeExcerpt.test.tsx

@@ -1,5 +1,6 @@
 import * as React from 'react'
 import _VisibilitySensor from 'react-visibility-sensor'
+import sinon from 'sinon'
 export class MockVisibilitySensor extends React.Component<{ onChange?: (isVisible: boolean) => void }> {
     constructor(props: { onChange?: (isVisible: boolean) => void }) {
         super(props)
@@ -23,7 +24,9 @@ jest.mock(
 )
 
 import { cleanup, getAllByText, getByText, render } from 'react-testing-library'
+import { of } from 'rxjs'
 import {
+    HIGHLIGHTED_FILE_LINES,
     HIGHLIGHTED_FILE_LINES_REQUEST,
     HIGHLIGHTED_FILE_LINES_SIMPLE_REQUEST,
 } from '../../../web/src/search/testHelpers'
@@ -104,4 +107,24 @@ describe('CodeExcerpt', () => {
             expect(span.textContent === 'test')
         }
     })
+
+    it('does not disable the highlighting timeout', () => {
+        /*
+            Because disabling the timeout should only ever be done in response
+            to the user asking us to do so, something that we do not do for
+            code excerpts because falling back to plaintext rendering is most
+            ideal.
+        */
+        const fetchHighlightedFileLines = sinon.spy(ctx => of(HIGHLIGHTED_FILE_LINES))
+        const highlightRange = [{ line: 12, character: 7, highlightLength: 4 }]
+        render(
+            <CodeExcerpt
+                {...defaultProps}
+                highlightRanges={highlightRange}
+                fetchHighlightedFileLines={fetchHighlightedFileLines}
+            />
+        )
+        sinon.assert.calledOnce(fetchHighlightedFileLines)
+        sinon.assert.calledWithMatch(fetchHighlightedFileLines, { disableTimeout: false })
+    })
 })

shared/src/components/CodeExcerpt.tsx

@@ -66,7 +66,7 @@ export class CodeExcerpt extends React.PureComponent<Props, State> {
                             commitID,
                             filePath,
                             isLightTheme,
-                            disableTimeout: true,
+                            disableTimeout: false,
                         })
                     ),
                     catchError(error => [asError(error)])

web/src/repo/backend.tsx

@@ -218,9 +218,6 @@ export const fetchHighlightedFileLines = memoizeObservable(
                 if (result.isDirectory) {
                     return []
                 }
-                if (result.highlightedFile.aborted) {
-                    throw new Error('aborted fetching highlighted contents')
-                }
                 let parsed = result.highlightedFile.html.substr('<table>'.length)
                 parsed = parsed.substr(0, parsed.length - '</table>'.length)
                 const rows = parsed.split('</tr>')