fix: action not found should return 404

[alexbjorlig]

When posting to a form action, that does not exist, the server returned 500(!)
With this fix, it will return 404 error "not found".

The problem with returning 500 is that it's the wrong error message and not something that should be reported to for example Sentry.

This is something I found, in relation to discussion 11248.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

[changeset-bot]

🦋 Changeset detected

Latest commit: 56a28f0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[benmccann]

Man. We're really having trouble with 500 vs 404. I think #11131 and #10565 addressed similar issues

[alexbjorlig]

Man. We're really having trouble with 500 vs 404. I think #11131 and #10565 addressed similar issues

I think you are correct - I was also a bit surprised that this was un-tested territory, but I'm all up for the task of adding more tests.

In my work, we are currently in the process of being SOC 2 certified, and we use an application scanner that generates a bunch of frustrating 500 errors when it's running against Sveltekit.

[eltigerchino]

Should we be using

kit/packages/kit/src/runtime/control.js

Line 33 in 108a6a8

export class NotFound extends Error {

instead of the error helper method?

[dummdidumm]

If we're being pedantic we can't really use error inside our code, ever. People could've added more properties to App.Error which means it's required to call error with more than a string. The combination of an error thrown by us and someone actually enhancing App.Error in a way that would cause breakage (beyond just showing undefined somewhere) is probably rare enough that it didn't occur in practise yet, but it's still a theoretical bug.

@alexbjorlig when you're saying that Sentry shouldn't be flooded by this, do you mean that you don't want handleError to be called, or that the status should be something else than 500?

I'm inclined to merge this, but clean it up for 2.0 (or 3.0, depending on how urgent we see this).

[alexbjorlig]

If we're being pedantic we can't really use error inside our code, ever. People could've added more properties to App.Error which means it's required to call error with more than a string. The combination of an error thrown by us and someone actually enhancing App.Error in a way that would cause breakage (beyond just showing undefined somewhere) is probably rare enough that it didn't occur in practise yet, but it's still a theoretical bug.

@alexbjorlig when you're saying that Sentry shouldn't be flooded by this, do you mean that you don't want handleError to be called, or that the status should be something else than 500?

I'm inclined to merge this, but clean it up for 2.0 (or 3.0, depending on how urgent we see this).

I have so many ideas for this discussion 🤔 Should we maybe move this to a dedicated discussion (instead of here on the PR)?

[dummdidumm]

I created #11287 for this, feel free to chime in!

[Rich-Harris]

If we're being pedantic we can't really use error inside our code, ever

And we weren't, prior to this PR and #11255. I'm inclined to revert both and revisit both issues in #11287.

[Rich-Harris]

Actually that's not quite true, there are a couple of other places where we're calling error internally. Those should be cleaned up as part of #11287