Missing `action` Function Returns 500 Instead of 405

[joshboley]

What version of Remix are you using?

1.13.0

Are all your remix dependencies & dev-dependencies using the same version?

• Yes

Steps to Reproduce

• Make any Remix route (page, resource, or api route) with an exported loader, but no action
• Make a GET request to the route, notice success
• Make a POST (or PUT, etc) request to the route, notice a 500 response

Example Route:

// routes/api/health.tsx
import {json} from '@remix-run/node'

export async function loader() {
  return json({
    status: 'success',
  })
}

Expected Behavior

During the non-GET request, POST in this example, a 405 status code should be returned, based on this discussion.

Actual Behavior

The returned status code is 500. Note however, that the Remix server outputs 405, but that is not the status code actually received by the caller.

Example curl:

curl -X POST http://localhost:3000/api/health -v

*   Trying 127.0.0.1:3000...
* Connected to localhost (127.0.0.1) port 3000 (#0)
> POST /api/health HTTP/1.1
> Host: localhost:3000
> User-Agent: curl/7.79.1
> Accept: */*
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 500 Internal Server Error
< content-type: text/plain; charset=utf-8
< Vary: Accept-Encoding
< Date: Fri, 17 Feb 2023 22:03:04 GMT
< Connection: keep-alive
< Keep-Alive: timeout=5
< Transfer-Encoding: chunked
<
Unexpected Server Error

* Connection #0 to host localhost left intact
[object Object]%

But note the output in the terminal from the running Remix server:

POST /api/health 500 - - 159.584 ms
ErrorResponse {
  status: 405,
  statusText: 'Method Not Allowed',
  internal: true,
  data: 'Error: You made a POST request to "/logout" but did not provide an `action` for route "routes/__app/logout", so there is no way to handle the request.',
  error: Error: You made a POST request to "/logout" but did not provide an `action` for route "routes/__app/logout", so there is no way to handle the request.

[machour]

@joshboley Can't reproduce on my side.
Would it be possible to share a reproduction on http://remix.new/ ?

[jazanne]

I am also seeing this issue in 1.14.0

[jazanne]

@machour Have done a bit of digging into this and it seems like what's happening is that the 405 error response is not be identified as a valid Response so a "last resort" error response is returned with 500 code.

Perhaps related to this change from December #4782

remix/packages/remix-server-runtime/server.ts

Lines 365 to 373 in a0341c9

	} catch (error: unknown) {
	if (isResponse(error)) {
	// Note: Not functionally required but ensures that our response headers
	// match identically to what Remix returns
	error.headers.set("X-Remix-Catch", "yes");
	return error;
	}
	return returnLastResortErrorResponse(error, serverMode);
	}

[joshboley]

@machour sorry for the late replay. I was able to reproduce this by using npx create-remix@latest with just the bare bones app. This only occurs when there is no default component being exported, meaning this only occurs for an API/Resource Route. The example in my original post is enough to reproduce the issue, but here's a link to a reproduction.

@jazanne thanks for digging into this and confirming!

[jazanne]

Any thoughts on whether or not this will be prioritized? It's a pretty disruptive issue for us since it skews the number of unexpected errors / 500s we see in monitoring.

[mfreundlich]

This is still reproducible on 1.15.0. Any word on this being prioritized? Like @jazanne mentioned, this leads to a large increase in 500s in monitoring.

[kiliman]

Here's a patch to fix the issue in Remix v1.15.0
https://gist.github.com/kiliman/a7d7e90d30b0d01e5ffe5426c9dfab69

diff --git a/node_modules/@remix-run/server-runtime/dist/responses.js b/node_modules/@remix-run/server-runtime/dist/responses.js
index d042d61..1e6d5c6 100644
--- a/node_modules/@remix-run/server-runtime/dist/responses.js
+++ b/node_modules/@remix-run/server-runtime/dist/responses.js
@@ -47,7 +47,7 @@ function isDeferredData(value) {
   return deferred && typeof deferred === "object" && typeof deferred.data === "object" && typeof deferred.subscribe === "function" && typeof deferred.cancel === "function" && typeof deferred.resolveData === "function";
 }
 function isResponse(value) {
-  return value != null && typeof value.status === "number" && typeof value.statusText === "string" && typeof value.headers === "object" && typeof value.body !== "undefined";
+  return value instanceof router.ErrorResponse || (value != null && typeof value.status === "number" && typeof value.statusText === "string" && typeof value.headers === "object" && typeof value.body !== "undefined");
 }
 const redirectStatusCodes = new Set([301, 302, 303, 307, 308]);
 function isRedirectStatusCode(statusCode) {
diff --git a/node_modules/@remix-run/server-runtime/dist/server.js b/node_modules/@remix-run/server-runtime/dist/server.js
index fd10bc5..b924771 100644
--- a/node_modules/@remix-run/server-runtime/dist/server.js
+++ b/node_modules/@remix-run/server-runtime/dist/server.js
@@ -110,6 +110,9 @@ async function handleDataRequestRR(serverMode, staticHandler, routeId, request,
     return response;
   } catch (error) {
     if (responses.isResponse(error)) {
+      if (error.headers === undefined) {
+        error.headers = new Headers()
+      }
       error.headers.set("X-Remix-Catch", "yes");
       return error;
     }
@@ -251,6 +254,9 @@ async function handleResourceRequestRR(serverMode, staticHandler, routeId, reque
     return response;
   } catch (error) {
     if (responses.isResponse(error)) {
+      if (error.headers === undefined) {
+        error.headers = new Headers()
+      }
       // Note: Not functionally required but ensures that our response headers
       // match identically to what Remix returns
       error.headers.set("X-Remix-Catch", "yes");