node/routes: Fix error handling

We should always send HTML if the user agent expects it.

If they do not, they should clearly indicate such via the Accept header.

Closes #2297

Author: nhooyr

src/node/routes/domainProxy.ts

@@ -45,11 +45,7 @@ router.all("*", (req, res, next) => {
       return next()
     }
 
-    // Assume anything that explicitly accepts text/html is a user browsing a
-    // page (as opposed to an xhr request). Don't use `req.accepts()` since
-    // *every* request that I've seen (in Firefox and Chromium at least)
-    // includes `*/*` making it always truthy.
-    if (typeof req.headers.accepts === "string" && req.headers.accepts.split(",").includes("text/html")) {
+    if (req.accepts("text/html")) {
       // Let the login through.
       if (/\/login\/?/.test(req.path)) {
         return next()

src/node/routes/index.ts

@@ -125,20 +125,15 @@ export const register = async (
     throw new HttpError("Not Found", HttpCode.NotFound)
   })
 
-  const errorHandler: express.ErrorRequestHandler = async (err, req, res) => {
+  const errorHandler: express.ErrorRequestHandler = async (err, req, res, next) => {
     if (err.code === "ENOENT" || err.code === "EISDIR") {
       err.status = HttpCode.NotFound
     }
 
     const status = err.status ?? err.statusCode ?? 500
     res.status(status)
 
-    if (req.accepts("application/json")) {
-      res.json({
-        error: err.message,
-        ...(err.details || {}),
-      })
-    } else {
+    if (req.accepts("text/html")) {
       const resourcePath = path.resolve(rootPath, "src/browser/pages/error.html")
       res.set("Content-Type", getMediaMime(resourcePath))
       const content = await fs.readFile(resourcePath, "utf8")
@@ -148,6 +143,11 @@ export const register = async (
           .replace(/{{ERROR_HEADER}}/g, status)
           .replace(/{{ERROR_BODY}}/g, err.message),
       )
+    } else {
+      res.json({
+        error: err.message,
+        ...(err.details || {}),
+      })
     }
   }