wrap async middlewares correctly (#28030)

* wrap async middlewares correctly

* clean up more

* feedbacked

middleware/catch-middleware-error.js

@@ -0,0 +1,3 @@
+export default function catchMiddlewareError(fn) {
+  return (req, res, next) => Promise.resolve(fn(req, res, next)).catch(next)
+}

middleware/contextualizers/features.js

@@ -1,6 +1,6 @@
 import getApplicableVersions from '../../lib/get-applicable-versions.js'
 
-export default async function features(req, res, next) {
+export default function features(req, res, next) {
   if (!req.context.page) return next()
 
   // Determine whether the currentVersion belongs to the list of versions the feature is available in.

middleware/contextualizers/short-versions.js

@@ -6,7 +6,7 @@
 // {% if ghes %}
 //
 // For the custom operator handling in statements like {% if ghes > 3.0 %}, see `lib/liquid-tags/if-ver.js`.
-export default async function shortVersions(req, res, next) {
+export default function shortVersions(req, res, next) {
   const { allVersions, currentVersion } = req.context
   const currentVersionObj = allVersions[currentVersion]
   if (!currentVersionObj) return next()

middleware/events.js

@@ -3,6 +3,7 @@ import { omit } from 'lodash-es'
 import Ajv from 'ajv'
 import addFormats from 'ajv-formats'
 import { eventSchema, hydroNames } from '../lib/schema-event.js'
+import catchMiddlewareError from './catch-middleware-error.js'
 
 const OMIT_FIELDS = ['type']
 
@@ -11,23 +12,26 @@ addFormats(ajv)
 
 const router = express.Router()
 
-router.post('/', async function postEvents(req, res, next) {
-  const isDev = process.env.NODE_ENV === 'development'
-  const fields = omit(req.body, '_csrf')
+router.post(
+  '/',
+  catchMiddlewareError(async function postEvents(req, res, next) {
+    const isDev = process.env.NODE_ENV === 'development'
+    const fields = omit(req.body, '_csrf')
 
-  if (!ajv.validate(eventSchema, fields)) {
-    return res.status(400).json(isDev ? ajv.errorsText() : {})
-  }
-
-  res.json({})
-
-  if (req.hydro.maySend()) {
-    try {
-      await req.hydro.publish(hydroNames[fields.type], omit(fields, OMIT_FIELDS))
-    } catch (err) {
-      console.error('Failed to submit event to Hydro', err)
+    if (!ajv.validate(eventSchema, fields)) {
+      return res.status(400).json(isDev ? ajv.errorsText() : {})
     }
-  }
-})
+
+    res.json({})
+
+    if (req.hydro.maySend()) {
+      try {
+        await req.hydro.publish(hydroNames[fields.type], omit(fields, OMIT_FIELDS))
+      } catch (err) {
+        console.error('Failed to submit event to Hydro', err)
+      }
+    }
+  })
+)
 
 export default router

middleware/find-page.js

@@ -1,7 +1,7 @@
 import getRedirect from '../lib/get-redirect.js'
 // This middleware uses the request path to find a page in the preloaded context.pages object
 
-export default async function findPage(req, res, next) {
+export default function findPage(req, res, next) {
   let page = req.context.pages[req.pagePath]
 
   // When a user navigates to a translated page that doesn't yet exists

middleware/handle-next-data-path.js

@@ -1,4 +1,4 @@
-export default async function handleNextDataPath(req, res, next) {
+export default function handleNextDataPath(req, res, next) {
   if (req.path.startsWith('/_next/data') && req.path.endsWith('.json')) {
     // translate a nextjs data request to a page path that the server can use on context
     // this is triggered via client-side route tranistions

middleware/index.js

@@ -203,7 +203,7 @@ export default function (app) {
   // *** Early exits ***
   app.get('/', fastRootRedirect)
   app.use(instrument(handleInvalidPaths, './handle-invalid-paths'))
-  app.use(asyncMiddleware(instrument(handleNextDataPath, './handle-next-data-path')))
+  app.use(instrument(handleNextDataPath, './handle-next-data-path'))
 
   // *** Security ***
   app.use(cors)
@@ -230,7 +230,7 @@ export default function (app) {
   app.use(recordRedirect)
   app.use(instrument(detectLanguage, './detect-language')) // Must come before context, breadcrumbs, find-page, handle-errors, homepages
   app.use(asyncMiddleware(instrument(context, './context'))) // Must come before early-access-*, handle-redirects
-  app.use(asyncMiddleware(instrument(shortVersions, './contextualizers/short-versions'))) // Support version shorthands
+  app.use(instrument(shortVersions, './contextualizers/short-versions')) // Support version shorthands
 
   // Must come before handleRedirects.
   // This middleware might either redirect to serve something.
@@ -244,19 +244,19 @@ export default function (app) {
   app.use(instrument(handleRedirects, './redirects/handle-redirects')) // Must come before contextualizers
 
   // *** Config and context for rendering ***
-  app.use(asyncMiddleware(instrument(findPage, './find-page'))) // Must come before archived-enterprise-versions, breadcrumbs, featured-links, products, render-page
+  app.use(instrument(findPage, './find-page')) // Must come before archived-enterprise-versions, breadcrumbs, featured-links, products, render-page
   app.use(instrument(blockRobots, './block-robots'))
 
   // Check for a dropped connection before proceeding
   app.use(haltOnDroppedConnection)
 
   // *** Rendering, 2xx responses ***
-  app.use('/events', asyncMiddleware(instrument(events, './events')))
-  app.use('/search', asyncMiddleware(instrument(search, './search')))
-  app.use('/healthz', asyncMiddleware(instrument(healthz, './healthz')))
-  app.use('/anchor-redirect', asyncMiddleware(instrument(anchorRedirect, './anchor-redirect')))
-  app.get('/_ip', asyncMiddleware(instrument(remoteIP, './remoteIP')))
-  app.get('/_build', asyncMiddleware(instrument(buildInfo, './buildInfo')))
+  app.use('/events', instrument(events, './events'))
+  app.use('/search', instrument(search, './search'))
+  app.use('/healthz', instrument(healthz, './healthz'))
+  app.use('/anchor-redirect', instrument(anchorRedirect, './anchor-redirect'))
+  app.get('/_ip', instrument(remoteIP, './remoteIP'))
+  app.get('/_build', instrument(buildInfo, './buildInfo'))
 
   // Check for a dropped connection before proceeding (again)
   app.use(haltOnDroppedConnection)
@@ -292,7 +292,7 @@ export default function (app) {
   app.use(instrument(currentProductTree, './contextualizers/current-product-tree'))
   app.use(asyncMiddleware(instrument(genericToc, './contextualizers/generic-toc')))
   app.use(asyncMiddleware(instrument(breadcrumbs, './contextualizers/breadcrumbs')))
-  app.use(asyncMiddleware(instrument(features, './contextualizers/features')))
+  app.use(instrument(features, './contextualizers/features'))
   app.use(asyncMiddleware(instrument(productExamples, './contextualizers/product-examples')))
 
   app.use(asyncMiddleware(instrument(featuredLinks, './featured-links')))

middleware/remote-ip.js

@@ -2,7 +2,7 @@ import { cacheControlFactory } from './cache-control.js'
 
 const noCacheControl = cacheControlFactory(0)
 
-export default async function remoteIp(req, res, next) {
+export default function remoteIp(req, res, next) {
   noCacheControl(res)
   res.json({
     ip: req.ip,

middleware/search.js

@@ -3,6 +3,7 @@ import libLanguages from '../lib/languages.js'
 import searchVersions from '../lib/search/versions.js'
 import loadLunrResults, { QueryTermError } from '../lib/search/lunr-search.js'
 import { cacheControlFactory } from './cache-control.js'
+import catchMiddlewareError from './catch-middleware-error.js'
 
 const languages = new Set(Object.keys(libLanguages))
 const versions = new Set(Object.values(searchVersions))
@@ -10,49 +11,52 @@ const router = express.Router()
 const cacheControl = cacheControlFactory(60 * 60 * 24)
 const noCacheControl = cacheControlFactory(0)
 
-router.get('/', async function getSearch(req, res, next) {
-  const { query, version, language, filters, limit: limit_ } = req.query
-  const limit = Math.min(parseInt(limit_, 10) || 10, 100)
-  if (!versions.has(version)) {
-    return res.status(400).json({ error: 'Unrecognized version' })
-  }
-  if (!languages.has(language)) {
-    return res.status(400).json({ error: 'Unrecognized language' })
-  }
-  if (!query || !limit) {
-    return res.status(200).json([])
-  }
-
-  try {
-    const results = await loadLunrResults({
-      version,
-      language,
-      query: `${query} ${filters || ''}`,
-      limit,
-    })
-    // Only reply if the headers have not been sent and the request was not aborted...
-    if (!res.headersSent && !req.aborted) {
-      cacheControl(res)
-
-      // Undo the cookie setting that CSRF sets.
-      // Otherwise it can't be cached in the CDN.
-      res.removeHeader('set-cookie')
-
-      return res.status(200).json(results)
+router.get(
+  '/',
+  catchMiddlewareError(async function getSearch(req, res, next) {
+    const { query, version, language, filters, limit: limit_ } = req.query
+    const limit = Math.min(parseInt(limit_, 10) || 10, 100)
+    if (!versions.has(version)) {
+      return res.status(400).json({ error: 'Unrecognized version' })
     }
-  } catch (err) {
-    if (err instanceof QueryTermError) {
-      // Handled as an not entirely unexpected potential error
-      return res.status(400).json({ error: err.toString() })
+    if (!languages.has(language)) {
+      return res.status(400).json({ error: 'Unrecognized language' })
+    }
+    if (!query || !limit) {
+      return res.status(200).json([])
     }
 
-    console.error(err)
-    // Only reply if the headers have not been sent and the request was not aborted...
-    if (!res.headersSent && !req.aborted) {
-      noCacheControl(res)
-      return res.status(400).json({ error: err.toString() })
+    try {
+      const results = await loadLunrResults({
+        version,
+        language,
+        query: `${query} ${filters || ''}`,
+        limit,
+      })
+      // Only reply if the headers have not been sent and the request was not aborted...
+      if (!res.headersSent && !req.aborted) {
+        cacheControl(res)
+
+        // Undo the cookie setting that CSRF sets.
+        // Otherwise it can't be cached in the CDN.
+        res.removeHeader('set-cookie')
+
+        return res.status(200).json(results)
+      }
+    } catch (err) {
+      if (err instanceof QueryTermError) {
+        // Handled as an not entirely unexpected potential error
+        return res.status(400).json({ error: err.toString() })
+      }
+
+      console.error(err)
+      // Only reply if the headers have not been sent and the request was not aborted...
+      if (!res.headersSent && !req.aborted) {
+        noCacheControl(res)
+        return res.status(400).json({ error: err.toString() })
+      }
     }
-  }
-})
+  })
+)
 
 export default router