Timeout requests and stop processing further

[MadLittleMods]

Timeout requests and stop processing further

Fix #148
Fix #40

• Apply timeout middleware to all room directory and room routes
• Stop messing with the response after we timeout. Fix Apply timeout middleware to all routes #148
  • This also involves cancelling any async/await things like requests in the routes so we throw an abort error instead of continuing on. Fix Cancellable requests #40
• Also abort the route if we see that the user closed the request before we could respond to them
• Bumps minimum supported Node.js version to v18 because we're now using the built-in native fetch in Node.js vs node-fetch. This gives us the custom signal.reason that we aborted with instead of a generic AbortError.
  • This also means we had to add some instrumentation for fetch which uses undici under the hood. Settled on some unofficial instrumentation: opentelemetry-instrumentation-fetch-node

Dev notes

Avoid errors like the following where the route function still continued on after we sent a timeout response to the user:

Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client
    at new NodeError (node:internal/errors:371:5)
    at ServerResponse.setHeader (node:_http_outgoing:576:11)
    at ServerResponse.header (matrix-public-archive/node_modules/express/lib/response.js:776:10)
    at ServerResponse.append (matrix-public-archive/node_modules/express/lib/response.js:737:15)
    at setHeadersToPreloadAssets (matrix-public-archive/server/lib/set-headers-to-preload-assets.js:73:7)
    at matrix-public-archive/server/routes/room-routes.js:908:5

Instead, we are left with these errors logged to the console each time we timeout. It would be best if we could ignore these in an error handler (added as a TODO here)

AbortError: The user aborted a request.
    at abort (matrix-public-archive/node_modules/node-fetch/lib/index.js:1448:16)
    at AbortSignal.abortAndFinalize (matrix-public-archive/node_modules/node-fetch/lib/index.js:1463:4)
    at AbortSignal.[nodejs.internal.kHybridDispatch] (node:internal/event_target:647:20)
    at AbortSignal.dispatchEvent (node:internal/event_target:589:26)
    at abortSignal (node:internal/abort_controller:283:10)
    at AbortController.abort (node:internal/abort_controller:314:5)
    at Timeout.<anonymous> (matrix-public-archive/server/middleware/timeout-middleware.js:25:25)
    at listOnTimeout (node:internal/timers:559:17)
    at processTimers (node:internal/timers:502:7)

Update: Switched to custom errors and native fetch in Node.js which emit the custom signal.reason instead of a generic error. So we now get a lovely error like this:

RouteTimeoutAbortError: Timed out after 25000ms while trying to respond to route /r/too-many-messages-on-day:my.synapse.server/date/2022/11/16
    at Timeout.<anonymous> (server/middleware/timeout-middleware.js:28:7)
    at listOnTimeout (node:internal/timers:569:17)
    at process.processTimers (node:internal/timers:512:7)

Todo

• Support abortSignal in renderHydrogenToString(...)
• Add an error handler to ignore AbortError (see dev notes for example) which appear in the logs whenever we timeout. We don't need to see these in the logs

[MadLittleMods]

Maybe, it would be interesting to attach some more info about the long-running requests that took up all of the time (get the spans).

Future PR stuff though.

Perhaps, it would just be better to have per-request correlated logging and see the other info that way, #9

[MadLittleMods]

Created nodejs/node#47814 to track when we can stop doing this workaround