Merge pull request #20783 from github/repo-sync

repo sync

middleware/api/search.js

@@ -112,28 +112,29 @@ router.get(
       'version:legacy',
       `indexName:${indexName}`,
     ])
+    const options = {
+      indexName,
+      query,
+      page: 1,
+      sort: 'best',
+      size: limit,
+      debug: true,
+      includeTopics: true,
+      // The legacy search is used as an autocomplete. In other words,
+      // a debounce that sends the query before the user has had a
+      // chance to fully submit the search. That means if the user
+      // send the query 'google cl' they hope to find 'Google Cloud'
+      // even though they didn't type that fully.
+      usePrefixSearch: true,
+    }
     try {
-      const searchResults = await timed({
-        indexName,
-        query,
-        page: 1,
-        sort: 'best',
-        size: limit,
-        debug: true,
-        includeTopics: true,
-        // The legacy search is used as an autocomplete. In other words,
-        // a debounce that sends the query before the user has had a
-        // chance to fully submit the search. That means if the user
-        // send the query 'google cl' they hope to find 'Google Cloud'
-        // even though they didn't type that fully.
-        usePrefixSearch: true,
-      })
+      const searchResults = await timed(options)
       hits.push(...searchResults.hits)
-    } catch (err) {
+    } catch (error) {
       // If we don't catch here, the `catchMiddlewareError()` wrapper
       // will take any thrown error and pass it to `next()`.
-      console.error('Error wrapping getSearchResults()', err)
-      return res.status(500).json([])
+      await handleGetSearchResultsError(req, res, error, options)
+      return
     }
 
     // The legacy search just returned an array
@@ -252,8 +253,9 @@ router.get(
       `indexName:${indexName}`,
     ])
 
+    const options = { indexName, query, page, size, debug, sort }
     try {
-      const { meta, hits } = await timed({ indexName, query, page, size, debug, sort })
+      const { meta, hits } = await timed(options)
 
       if (process.env.NODE_ENV !== 'development') {
         // The assumption, at the moment is that searches are never distinguished
@@ -271,28 +273,27 @@ router.get(
       // 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 {
-        const reports = FailBot.report(error, {
-          url: req.url,
-          indexName,
-          query,
-          page,
-          size,
-          debug,
-          sort,
-        })
-        // It might be `undefined` if no backends are configured which
-        // is likely when using production NODE_ENV on your laptop
-        // where you might not have a HATSTACK_URL configured.
-        if (reports) await Promise.all(reports)
-      }
-      res.status(500).send(error.message)
+      await handleGetSearchResultsError(req, res, error, options)
     }
   })
 )
 
+// We have more than one place where we do `try{...} catch error( THIS )`
+// which is slightly different depending on the "sub-version" (e.g. /legacy)
+// This function is a single place to take care of all of these error handlings
+async function handleGetSearchResultsError(req, res, error, options) {
+  if (process.env.NODE_ENV === 'development') {
+    console.error(`Error calling getSearchResults(${options})`, error)
+  } else {
+    const reports = FailBot.report(error, Object.assign({ url: req.url }, options))
+    // It might be `undefined` if no backends are configured which
+    // is likely when using production NODE_ENV on your laptop
+    // where you might not have a HATSTACK_URL configured.
+    if (reports) await Promise.all(reports)
+  }
+  res.status(500).send(error.message)
+}
+
 // Alias for the latest version
 router.get('/', (req, res) => {
   // At the time of writing, the latest version is v1. (July 2022)

middleware/next.js

@@ -8,6 +8,14 @@ export const nextHandleRequest = nextApp.getRequestHandler()
 await nextApp.prepare()
 
 function renderPageWithNext(req, res, next) {
+  // We currently don't use next/image for any images.
+  // We don't even have `sharp` installed.
+  // This could change in the future but right now can just 404 on these
+  // so we don't have to deal with any other errors.
+  if (req.path.startsWith('/_next/image')) {
+    return next(404)
+  }
+
   const isNextDataRequest = req.path.startsWith('/_next') && !req.path.startsWith('/_next/data')
 
   if (isNextDataRequest) {

tests/routing/next.js

@@ -0,0 +1,12 @@
+import { describe, expect, jest, test } from '@jest/globals'
+
+import { get } from '../helpers/e2etest.js'
+
+describe('redirects', () => {
+  jest.setTimeout(60 * 1000)
+
+  test('any _next/image request should 404', async () => {
+    const res = await get('/_next/image?what=ever')
+    expect(res.statusCode).toBe(404)
+  })
+})