pages respond with 500 status if missing action

[Graham42]

For the home page of my site I only want to support GET requests, but if a bot comes along and does a drive-by POST request, then I end up with a 500 status response being returned and a stacktrace in my logs. This adds noise and false positives to monitoring which makes it harder to detect real issues. The 500 status code should be reserved for unexpected errors, and in the case of a POST request to / I intend to not support that method.

For the first approach I tried adding an action function the returned a simple response with a 405 (Method Not Allowed) status

export let action: ActionFunction = async () => {
  return new Response(undefined, {
    status: 405,
    headers: {
      Allow: "GET"
    }
  })
}

But when I make the POST request, the result is a 500 internal server error and the log error is:

Error! Failed to complete request to /: Error: socket hang up
Unhandled rejection: Error: You made a POST request to http://localhost:3000/ but did not return a redirect. Please `return redirect(newUrl)` from your `action` function to avoid reposts when users click the back button.

This makes lots of sense for a form, but for a page where I expect to never recieve a POST, PUT, etc I don't think it applies.

For my next approach I tried what feels like a bit of a 🐒 🔧 monkey patch as it uses remix internals. In the server/index.js file I've set up a handler to wrap the remix handler which checks if there's a route match (I think I did this wrong) and if there is, checks if there's an action for the route. If there's no action it then sends a 405 response. Otherwise it falls back to the remix handler. ⚠️ This code ends up always returning 405s though, and not 404s because it doesn't detect route matching correctly.

server/index.js:

const { createRequestHandler } = require("@remix-run/vercel");
const { matchServerRoutes } = require("@remix-run/node/routeMatching");
const { createRoutes } = require("@remix-run/node/routes");

const build = require("./build");

const remixHandler = createRequestHandler({
  build: build,
});

const routes = createRoutes(build.routes);

/** @type {import("@remix-run/vercel").RequestHandler} */
const handler = async (req, res) => {
  let method = req.method.toUpperCase();
  // Everything except GET
  if (["HEAD", "OPTIONS", "POST", "PUT", "PATCH", "DELETE"].includes(method)) {
    let matches = matchServerRoutes(routes, req.url);
    // If there's no match for this route, the remix handler will return 404,
    // which is good. This is just to hand the case where the route matches, but
    // there's no action handler defined.
    if (matches || matches.length < 1) {
      let routeMatch = matches[matches.length - 1];
      console.log("matches: " + routeMatch.pathname);
      if (!routeMatch.route.module.action) {
        // Log an error for visibility. This is better than the default
        // behaviour of crashing the lambda which results in a future cold start
        console.error(
          `No action defined for ${req.method} request to ${req.url}`
        );
        // Method Not Allowed
        res.status(405);
        // This list is the inverse of above
        res.setHeader("Allow", "GET, HEAD");
        res.send();
        return;
      }
    }
  }

  await remixHandler(req, res);
};

module.exports = handler;

Maybe it would be helpful to have an option for a default action handler?

To restate my goal here: I would like to not see responses with a status code of 500 unless there's a real error in my code that I need to investigate. This is important for monitoring the health of a site.

Thanks for your time reading this post!

[machour]

You raise a valid concern, as it differs from the way a classical HTTP server would handle this.

I'm wondering what way the fix would go:

• align with classical web servers: make handleDataRequest treat POST|PUT|PATCH|DELETE as a GET when there's no action() exported
• change the 500 to a 405 (Method Not Allowed) and document this behavior

[ryanflorence]

we should change it to 405, and then consider some generic way to handle this in the future.

[machour]

Fixed in #2342