Merge pull request github#20435 from github/repo-sync

repo sync

Author: Octomerger

.github/workflows/test.yml

@@ -52,7 +52,7 @@ jobs:
         if: ${{ matrix.test-group == 'content' }}
         uses: getong/elasticsearch-action@95b501ab0c83dee0aac7c39b7cea3723bef14954
         with:
-          elasticsearch version: '7.17.5'
+          elasticsearch version: '7.11.1'
           host port: 9200
           container port: 9200
           host node port: 9300

middleware/api/es-search.js

@@ -48,22 +48,24 @@ export async function getSearchResults({
   const highlight = getHighlightConfiguration(query)
 
   const searchQuery = {
-    index: indexName,
     highlight,
     from,
     size,
-    // Since we know exactly which fields from the source we're going
-    // need we can specify that here. It's an inclusion list.
-    // We can save precious network by not having to transmit fields
-    // stored in Elasticsearch to here if it's not going to be needed
-    // anyway.
-    _source_includes: [
-      'title',
-      'url',
-      'breadcrumbs',
-      // 'headings'
-      'popularity',
-    ],
+
+    // COMMENTED out because of ES 7.11.
+    // Once we're on ES >7.11  we can add this option in.
+    // // Since we know exactly which fields from the source we're going
+    // // need we can specify that here. It's an inclusion list.
+    // // We can save precious network by not having to transmit fields
+    // // stored in Elasticsearch to here if it's not going to be needed
+    // // anyway.
+    // _source_includes: [
+    //   'title',
+    //   'url',
+    //   'breadcrumbs',
+    //   // 'headings'
+    //   'popularity',
+    // ],
   }
 
   if (includeTopics) {
@@ -107,15 +109,17 @@ export async function getSearchResults({
     throw new Error(`Unrecognized sort enum '${sort}'`)
   }
 
-  const result = await client.search(searchQuery)
+  const result = await client.search({ index: indexName, body: searchQuery })
 
-  const hits = getHits(result.hits.hits, { indexName, debug, includeTopics })
+  // const hitsAll = result.hits  // ES >7.11
+  const hitsAll = result.body // ES <=7.11
+  const hits = getHits(hitsAll.hits.hits, { indexName, debug, includeTopics })
   const t1 = new Date()
 
   const meta = {
-    found: result.hits.total,
+    found: hitsAll.hits.total,
     took: {
-      query_msec: result.took,
+      query_msec: hitsAll.took,
       total_msec: t1.getTime() - t0.getTime(),
     },
     page,

middleware/api/search.js

@@ -1,6 +1,7 @@
 import express from 'express'
 
 import searchVersions from '../../lib/search/versions.js'
+import FailBot from '../../lib/failbot.js'
 import languages from '../../lib/languages.js'
 import { allVersions } from '../../lib/all-versions.js'
 import { defaultCacheControl } from '../cache-control.js'
@@ -234,20 +235,42 @@ router.get(
   notConfiguredMiddleware,
   catchMiddlewareError(async function search(req, res, next) {
     const { indexName, query, page, size, debug, sort } = req.search
-    const { meta, hits } = await getSearchResults({ indexName, query, page, size, debug, sort })
+    try {
+      const { meta, hits } = await getSearchResults({ indexName, query, page, size, debug, sort })
 
-    if (process.env.NODE_ENV !== 'development') {
-      // The assumption, at the moment is that searches are never distinguished
-      // differently depending on a cookie or a request header.
-      // So the only distinguishing key is the request URL.
-      // Because of that, it's safe to allow the reverse proxy (a.k.a the CDN)
-      // cache and hold on to this.
-      defaultCacheControl(res)
-    }
+      if (process.env.NODE_ENV !== 'development') {
+        // The assumption, at the moment is that searches are never distinguished
+        // differently depending on a cookie or a request header.
+        // So the only distinguishing key is the request URL.
+        // Because of that, it's safe to allow the reverse proxy (a.k.a the CDN)
+        // cache and hold on to this.
+        defaultCacheControl(res)
+      }
 
-    // The v1 version of the output matches perfectly what comes out
-    // of the getSearchResults() function.
-    res.status(200).json({ meta, hits })
+      // The v1 version of the output matches perfectly what comes out
+      // of the getSearchResults() function.
+      res.status(200).json({ meta, hits })
+    } catch (error) {
+      // If getSearchResult() throws an error that might be 404 inside
+      // elasticsearch, if we don't capture that here, it will propgate
+      // to the next middleware.
+      if (process.env.NODE_ENV === 'development') {
+        console.error('Error calling getSearchResults()', error)
+      } else {
+        await Promise.all(
+          FailBot.report(error, {
+            url: req.url,
+            indexName,
+            query,
+            page,
+            size,
+            debug,
+            sort,
+          })
+        )
+      }
+      res.status(500).send(error.message)
+    }
   })
 )