kit: Don't automatically throw from `redirect` and `error`

I don’t have time to address everything here, but I did want to chime in on the “throw vs. redirect” thing – I used to be in the “Wow, I really wish we could just return these” camp. Unfortunately, the ergonomics of that solution in reality are just untenable. It seems “obvious” that this should work:

export function load() {
  return redirect('/foo');
}

But what happens if you have this?:

export const load = withAuthentication(() => {
  // get stuff from the database, realize something failed
  return error(400);
})

Now what? withAuthentication has to check the return value of your route-level load function and decide whether or not it should return early (if load returned an error or redirect) or if it should continue on. In complex apps, you could have several layers of this. Not to mention, it’s quite possible you want some code in /lib to be able to redirect users if, say, the user doesn’t have access to a resource. Without throw, all of their return types would have to be Whatever | error, which would then have to be passed allllll the way back up the stack until you could “give it back” to SvelteKit. Gross. throw is the control-flow paradigm you want for redirects and errors.

exceptions are not ideal for control flow

@FedericoBiccheddu As I understand it the anti-pattern is to abuse throw as a goto to jump around rather than using a more appropriate control structure.

In this case (both in the current API and in my proposed fix) throw is the appropriate control structure. When you do throw redirect(302, '/xyz) you’re saying “Hey, Kit, I’m done, go do your thing.”

You can achieve the same result with an early return. Using throw is literally using exception for control flow.

Exceptions are something you don’t expect in your program/logic. Saying “I’m done, now framework will do the rest” is not an exception, is exactly what do you expect from it.

To answer the authentication scenario, another alternative is to bump authentication to hooks.server.ts. That’s where I do my auth, and I prefer it there so that no unauthenticated actions accidentally slip through to a route inside. (This also saves me from having to add boiler plate to every route that I want protected.) I guess it depends on whether you want to be “safe first” or “open/public first”. If this risks making the handle function too complex, the logic related to auth within hooks.server.ts could be split into sub functions that are responsible for different sets of routes within the app.

I think the “‘Wow, I really wish we could just return these’ camp” is valid. Developers should be allowed to accomplish whatever goals they need. I believe Remix (which seems to have inspired SvelteKit, SolidStart, etc.) actually does this. You can return or throw regular Response objects depending on your needs, or you can use their helper functions. This is a better DX than having a set of options stripped from you completely, as in the case of SvelteKit and Next.js. (I’ve found the latter even less convenient.)

It’s really hard to predict every use case the developer needs to satisfy. @FedericoBiccheddu couldn’t predict @tcc-sejohnson’s use case/preference, who in turn likely can’t predict another set of use cases for returning redirect (because we haven’t each built the apps that have locked us down those paths). I actually have use cases for why I need to be able to return a regular Response object (which will be posted soon in a separate GH Issue).

Personally, I love Svelte. I think it’s much better than React. But I like Remix more than SvelteKit because I find myself butting heads with SvelteKit too often to accomplish what I want when it comes to headers, response objects, and cookies. In Remix, I just say, “Use this Response please” and everything works.

That said, regardless of whether we’re allowed to return Responses or not, I agree with @cdcarson’s reasons for not throwing implicitly. I also agree that there are situations where it would make sense to throw (if there’s a legitimate error).

If possible, I would completely remove the chance or need to throw something for control flow. There is a lot of literature that explains why exceptions are not ideal for control flow

Throwing something makes it very difficult to use other libraries in our backend logic. You have to add boilerplate to throw at the edge of the logic, resulting in more code, a non-linear flow, and patterns that are not widely used (like throwing for control flow itself).

My proposed solution is to have internal classes (like Response, Redirect, Error, and so on) and exported “constructors” (like response, redirect, error and so on) that hide the implementation detail of the classes that can be returned from loaders, actions and so on. Svelte can then pattern match the result, even with a simple instanceof, and decide what to do.

This would remove a lot of confusion about this pattern, use a well known approach by other frameworks, create reusable errors/redirects and simplify the interoperability with other libraries.

EDIT: seems that months ago this was the implementation and got changed. Is it possible to have an issue, PR or anything where the rationale behind this change is explained? I’m struggling to find resources about this topic.

@DePasqualeOrg Got it. I was using the SvelteKit default eslint, which I assumed had that rule, but maybe that’s because I nearly always explicitly type function returns. This has more or less the same effect as the rule, complaining if there’s a logical branch inside the function that doesn’t return the sanctified type. More to the point, SvelteKit shouldn’t add conventions that break cases like yours, especially on such apparently dubious grounds as those which led to this change.

I wanted to share my example of the API of redirect being inconsistent and confusing, and how that can lead to problems. In endpoints return redirect can be achieved multiple ways but I was not expecting form actions on post to work different then post even when using use: enhance @ITenthusiasm