throw redirect(status, url) should be return redirect(status, url) instead

[ghost]

for a scenario, using a try catch block where all exceptions are caught so that user is redirect back to a certain page, throw redirect(301, "/path") is creating issue, since this is also treated as an error.

current workaround is checking if error instanceof Response, but this is not a good method, instead of throwing redirect, returning redirect would perfectly solve the problem.

Haven't anyone felt this problem?? Or there's some better and I'm unknown about on redirects 😢

[vekunz]

I had this problem too, but switching from throw redirect() to return redirect() would have a big other issue: I think, it would be impossible for SvelteKit to differentiate between return redirect() and a normal return {status: 302, location: '/somewhere'}.

[cdcarson]

It was a mistake for SvelteKit to reverse the paradigm (in some cases) of "return success, throw errors."

I had this problem too, but switching from throw redirect() to return redirect() would have a big other issue: I think, it would be impossible for SvelteKit to differentiate between return redirect() and a normal return {status: 302, location: '/somewhere'}.

I'm pretty sure that's not the case. SvelteKit's error, fail, redirect, etc methods return classes. Example:

kit/packages/kit/src/exports/index.js

Lines 103 to 105 in b43ea97

	export function fail(status, data) {
	return new ActionFailure(status, data);
	}

This means Kit could very easily differentiate the various success and failure cases with instanceof checks in a try/catch. But I think it's kinda too late to fix -- it'd just cause more confusion.

[vekunz]

redirect() actually does not return a class, but an object {status: 302, location: '/somewhere'}.

[cdcarson]

Under the hood it's a class...

kit/packages/kit/src/exports/index.js

Lines 41 to 48 in f63a4b3

	export function redirect(status, location) {
	if ((!BROWSER || DEV) && (isNaN(status) || status < 300 || status > 308)) {
	throw new Error('Invalid status code');
	}
	return new Redirect(status, location);
	}

[vekunz]

That's weird, because in our unit tests, we check the throw-Result to be Equal {status: 302, location: '/somewhere'}. But maybe Vitest is just smart enough (or dumb enough, depends on point of view) to compare a class with an object.

[ghost]

so i guess, only way to handle redirects inside try catch is:

try {
    // do something
} catch(error) {
    if(error.hasOwnProperty("location")) throw error;
    // otherwise handle actual error :disappointed: 
}

[david-plugge]

import { type HttpError, type Redirect, error, redirect } from '@sveltejs/kit';

export function isKitError(e: unknown): e is HttpError {
	return e instanceof error(500).constructor;
}

export function isKitRedirect(e: unknown): e is Redirect {
	return e instanceof redirect(300, '/').constructor;
}

export function isKitThrowable(e: unknown): e is HttpError | Redirect {
	return isKitError(e) || isKitRedirect(e);
}

export function throwKitThrowable<T>(e: T): T extends HttpError | Redirect ? never : void {
	if (isKitThrowable(e)) throw e;
	// @ts-expect-error typescript is weird
	return;
}

[vekunz]

You should be able to make a shortcut:

import { HttpError, Redirect } from '@sveltejs/kit';

export function isKitError(e: unknown): e is HttpError {
	return e instanceof HttpError;
}

...

We can't use this way, because of some npm package magic stuff (we have multiple internal npm packages).

[david-plugge]

Ahh, last time i checked the classes were not exported, good to know.
So I assume the problem is that there are multiple definitions of HttpError and Redirect? That´s a weird thing...

[david-plugge]

Actually, no the classes are not exported, just the types. e instanceof HttpError results in TypeError: Right-hand side of 'instanceof' is not an object.

[vekunz]

Actually, no the classes are not exported, just the types. e instanceof HttpError results in TypeError: Right-hand side of 'instanceof' is not an object.

Ok, then I remembered it wrong

So I assume the problem is that there are multiple definitions of HttpError and Redirect? That´s a weird thing...

There is only definition of them, but in some npm package constellations this does not matter. We have this standard error handling in a private npm package (we have a company internal npm registry). If we install it normally, everything works fine. But if we install the package locally through the file system, this does not work, because then the SvelteKit package is installed first in our app and second in the package, and then they are two different definitions even there are virtually the same.