🧹 Remove connect-timeout and add per-fetch timeouts (#61086)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Author: 3 people

package-lock.json

@@ -198,7 +198,6 @@
     "cheerio": "^1.2.0",
     "classnames": "^2.5.1",
     "clsx": "^2.1.1",
-    "connect-timeout": "1.9.1",
     "cookie-parser": "^1.4.7",
     "cuss": "2.2.0",
     "dayjs": "^1.11.19",
@@ -282,7 +281,6 @@
     "@octokit/rest": "22.0.0",
     "@playwright/test": "^1.58.2",
     "@types/accept-language-parser": "1.5.7",
-    "@types/connect-timeout": "1.9.0",
     "@types/cookie": "0.6.0",
     "@types/cookie-parser": "1.4.8",
     "@types/eslint-plugin-jsx-a11y": "^6.10.1",

package.json

@@ -1,6 +1,8 @@
 /**
  * Copilot Space API response types
  */
+import { fetchWithRetry } from '@/frame/lib/fetch-utils'
+
 export interface SpaceResource {
   id: number
   resource_type: string
@@ -48,14 +50,28 @@ export function parseSpaceUrl(url: string): { org: string; id: string } {
 export async function fetchCopilotSpace(spaceUrl: string): Promise<SpaceData> {
   const { org, id } = parseSpaceUrl(spaceUrl)
   const apiUrl = `https://api.github.com/orgs/${org}/copilot-spaces/${id}`
+  const githubToken = process.env.GITHUB_TOKEN
+
+  if (!githubToken) {
+    throw new Error('GITHUB_TOKEN environment variable is required to fetch Copilot Spaces.')
+  }
 
-  const response = await fetch(apiUrl, {
-    headers: {
-      Authorization: `Bearer ${process.env.GITHUB_TOKEN}`,
-      Accept: 'application/vnd.github+json',
-      'X-GitHub-Api-Version': '2022-11-28',
+  const response = await fetchWithRetry(
+    apiUrl,
+    {
+      headers: {
+        Authorization: `Bearer ${githubToken}`,
+        Accept: 'application/vnd.github+json',
+        'X-GitHub-Api-Version': '2022-11-28',
+      },
+    },
+    {
+      retries: 0,
+      timeout: 30_000,
+      // We handle non-2xx responses below with status-specific messages.
+      throwHttpErrors: false,
     },
-  })
+  )
 
   if (!response.ok) {
     if (response.status === 404) {

src/ai-tools/lib/spaces-utils.ts

@@ -94,6 +94,9 @@ export default async function archivedEnterpriseVersionsAssets(
       {
         retries: 0,
         throwHttpErrors: true,
+        // Stay safely under MAX_REQUEST_TIMEOUT (10s) so the upstream
+        // can't push us past the request budget.
+        timeout: 8_000,
       },
     )
 

src/archives/middleware/archived-enterprise-versions-assets.ts

@@ -2,6 +2,9 @@
  * Utility functions for fetch with retry and timeout functionality
  * to replace got library functionality
  */
+import statsd from '@/observability/lib/statsd'
+
+const STATSD_FETCH_TIMEOUT = 'fetch.timeout'
 
 export interface FetchWithRetryOptions {
   retries?: number
@@ -52,6 +55,14 @@ async function fetchWithTimeout(
   } catch (error) {
     clearTimeout(timeoutId)
     if (error instanceof Error && error.name === 'AbortError') {
+      const host = (() => {
+        try {
+          return new URL(typeof url === 'string' ? url : url.toString()).host
+        } catch {
+          return 'unknown'
+        }
+      })()
+      statsd.increment(STATSD_FETCH_TIMEOUT, 1, [`host:${host}`])
       throw new Error(`Request timed out after ${timeout}ms`)
     }
     throw error

src/frame/lib/fetch-utils.ts

@@ -3,9 +3,7 @@ import path from 'path'
 
 import express from 'express'
 import type { NextFunction, Request, Response, Express } from 'express'
-import timeout from 'connect-timeout'
 
-import { haltOnDroppedConnection } from './halt-on-dropped-connection'
 import abort from './abort'
 import helmet from './helmet'
 import cookieParser from './cookie-parser'
@@ -65,14 +63,10 @@ import dynamicAssets from '@/assets/middleware/dynamic-assets'
 import generalSearchMiddleware from '@/search/middleware/general-search-middleware'
 import shielding from '@/shielding/middleware'
 import safeRedirect from './safe-redirect'
-import { MAX_REQUEST_TIMEOUT } from '@/frame/lib/constants'
 import { initLoggerContext } from '@/observability/logger/lib/logger-context'
 import { getAutomaticRequestLogger } from '@/observability/logger/middleware/get-automatic-request-logger'
 import urlDecode from './url-decode'
 
-const { NODE_ENV } = process.env
-const isTest = NODE_ENV === 'test' || process.env.GITHUB_ACTIONS === 'true'
-
 const ENABLE_FASTLY_TESTING = JSON.parse(process.env.ENABLE_FASTLY_TESTING || 'false')
 
 // Catch unhandled promise rejections and passing them to Express's error handler
@@ -90,8 +84,6 @@ const asyncMiddleware =
   }
 
 export default function index(app: Express) {
-  // *** Request connection management ***
-  if (!isTest) app.use(timeout(MAX_REQUEST_TIMEOUT))
   app.use(abort)
 
   // Don't use the proxy's IP, use the requester's for rate limiting or
@@ -237,9 +229,6 @@ export default function index(app: Express) {
   app.use(asyncMiddleware(findPage)) // Must come before archived-enterprise-versions, breadcrumbs, featured-links, products, render-page
   app.use(blockRobots)
 
-  // Check for a dropped connection before proceeding
-  app.use(haltOnDroppedConnection)
-
   // *** Rendering, 2xx responses ***
   app.use('/api', api)
   app.use('/llms.txt', llmsTxt)
@@ -251,17 +240,11 @@ export default function index(app: Express) {
   // Now that the `req.language` is known, set it for the remaining endpoints
   app.use(setLanguageFastlySurrogateKey)
 
-  // Check for a dropped connection before proceeding (again)
-  app.use(haltOnDroppedConnection)
-
   app.use(robots)
   app.use(earlyAccessLinks)
   app.use('/categories.json', asyncMiddleware(categoriesForSupport))
   app.get('/_500', asyncMiddleware(triggerError))
 
-  // Check for a dropped connection before proceeding (again)
-  app.use(haltOnDroppedConnection)
-
   // Specifically deal with HEAD requests before doing the slower
   // full page rendering.
   app.head('/*path', fastHead)
@@ -292,9 +275,6 @@ export default function index(app: Express) {
   // handle serving NextJS bundled code (/_next/*)
   app.use(next)
 
-  // Check for a dropped connection before proceeding (again)
-  app.use(haltOnDroppedConnection)
-
   // *** Rendering, must go almost last ***
   app.get('/*path', asyncMiddleware(renderPage))
 

src/frame/middleware/halt-on-dropped-connection.ts

@@ -13,7 +13,6 @@ import { allVersions } from '@/versions/lib/all-versions'
 import { transformerRegistry } from '@/article-api/transformers'
 import { minimumNotFoundHtml } from '../lib/constants'
 import { contentTypeCacheControl, defaultCacheControl } from './cache-control'
-import { isConnectionDropped } from './halt-on-dropped-connection'
 import { nextHandleRequest } from './next'
 
 const logger = createLogger(import.meta.url)
@@ -98,9 +97,6 @@ export default async function renderPage(req: ExtendedRequest, res: Response) {
     res.setHeader('Last-Modified', new Date(page.effectiveDate).toUTCString())
   }
 
-  // Stop processing if the connection was already dropped
-  if (isConnectionDropped(req, res)) return
-
   // Content negotiation: serve markdown when the client prefers it over HTML.
   // Agents like Claude Code send Accept headers that omit text/html.
   if (req.accepts(['text/html', 'text/markdown']) === 'text/markdown') {
@@ -123,9 +119,6 @@ export default async function renderPage(req: ExtendedRequest, res: Response) {
     req.context.miniTocItems = buildMiniTocItems(req)
   }
 
-  // Stop processing if the connection was already dropped
-  if (isConnectionDropped(req, res)) return
-
   // Create string for <title> tag
   page.fullTitle = page.title
 

src/frame/middleware/index.ts

@@ -5,7 +5,6 @@ import { nextApp } from '@/frame/middleware/next'
 import { minimumNotFoundHtml } from '@/frame/lib/constants'
 import { setFastlySurrogateKey, SURROGATE_ENUMS } from '@/frame/middleware/set-fastly-surrogate-key'
 import { errorCacheControl } from '@/frame/middleware/cache-control'
-import statsd from '@/observability/lib/statsd'
 import { toError } from '@/observability/lib/to-error'
 import { ExtendedRequest } from '@/types'
 import { createLogger } from '@/observability/logger'
@@ -43,33 +42,12 @@ async function logException(error: ErrorWithCode, req: ExtendedRequest) {
   }
 }
 
-function timedOut(req: ExtendedRequest) {
-  // The `req.pagePath` can come later so it's not guaranteed to always
-  // be present. It's added by the `handle-next-data-path.ts` middleware
-  // we translates those "cryptic" `/_next/data/...` URLs from
-  // client-side routing.
-  const incrementTags = [`path:${req.pagePath || req.path}`]
-  if (req.context?.currentCategory) {
-    incrementTags.push(`product:${req.context.currentCategory}`)
-  }
-  statsd.increment('middleware.timeout', 1, incrementTags)
-  logger.warn('Request timed out', {
-    path: req.pagePath || req.path,
-    method: req.method,
-  })
-}
-
 async function handleError(
   error: ErrorWithCode | number,
   req: ExtendedRequest,
   res: Response,
   next: NextFunction,
 ) {
-  // Potentially set by the `connect-timeout` middleware.
-  if (req.timedout) {
-    timedOut(req)
-  }
-
   const responseDone = res.headersSent || req.aborted
 
   if (req.path.startsWith('/assets') || req.path.startsWith('/_next/static')) {