Repository: sveltejs/kit (js)

Issue:
Clientside link navigation doesn't respect redirects
### Describe the bug

When using SvelteKit's clientside link navigation, redirects are not being respected unless the link has the `data-sveltekit-reload` attribute. If a page returns a redirect, Kit just throws a 500 Internal Error for the user (no message on the server). You can verify that the responses are getting sent correctly and have 3xx status codes by checking devtools.

This happens both when redirecting from `hooks.server.ts` and `+page.server.ts`.

When using the `data-sveltekit-reload` or manually refreshing the page, the redirect works fine.

It doesn't seem to matter how the redirect is sent: Kit's `redirect()` error, `Response(null, { status: 302, headers: { Location: '/login' } })`, and `Response.redirect(event.url.origin + '/login', 302);` all give the same result.

I couldn't find any docs on this, so I assume this is either unintended behavior or I'm doing something wrong.

### Reproduction

https://github.com/styxpilled/sveltekit-navigation-redirect-issue

### Logs

_No response_

### System Info

```Shell
System:
    OS: Windows 10 10.0.19044
    CPU: (4) x64 Intel(R) Core(TM) i3-9100F CPU @ 3.60GHz
    Memory: 453.96 MB / 15.94 GB
  Binaries:
    Node: 18.11.0 - ~\AppData\Local\pnpm\node.EXE
    npm: 8.19.2 - ~\AppData\Local\pnpm\npm.CMD
  Browsers:
    Edge: Spartan (44.19041.1266.0), Chromium (107.0.1418.56)
    Internet Explorer: 11.0.19041.1566
```


### Severity

annoyance

### Additional Information

_No response_

Pull request description:
[fix] handle redirects in handle hook while processing data request
Fixes #7787

During fixing this, I wondered why the tests also pass without the fix, and discovered that this specific behavior will only occur if there's no root `+layout.server.js` (because then it wouldn't try to rerequest the data, failing again, resulting in a fallback to the server where the redirect would be handled correctly), which we have in the tests. I'm not sure if it's worth creating another test suite for this, so I left it like this.

### Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
- [x] 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
- [x] This message body should clearly illustrate what problems it solves.
- [x] Ideally, include a test that fails without this PR but passes with it.

### Tests
- [x] Run the tests with `pnpm test` and lint the project with `pnpm lint` and `pnpm check`

### Changesets
- [x] 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. All changesets should be `patch` until SvelteKit 1.0


Patch:
diff --git a/.changeset/calm-peaches-enjoy.md b/.changeset/calm-peaches-enjoy.md
new file mode 100644
index 000000000..aafb78f21
--- /dev/null
+++ b/.changeset/calm-peaches-enjoy.md
@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+[fix] handle redirects in handle hook while processing data request
diff --git a/.changeset/pre.json b/.changeset/pre.json
index 3eee6eb45..8a1747b83 100644
--- a/.changeset/pre.json
+++ b/.changeset/pre.json
@@ -819,6 +819,7 @@
     "ninety-colts-protect",
     "ninety-dodos-exercise",
     "ninety-dolls-sniff",
+    "ninety-ducks-collect",
     "ninety-fans-cover",
     "ninety-games-flow",
     "ninety-glasses-exercise",
diff --git a/documentation/docs/30-advanced/20-hooks.md b/documentation/docs/30-advanced/20-hooks.md
index 62fa4a564..a07624fc1 100644
--- a/documentation/docs/30-advanced/20-hooks.md
+++ b/documentation/docs/30-advanced/20-hooks.md
@@ -72,7 +72,7 @@ You can add call multiple `handle` functions with [the `sequence` helper functio
 
 - `transformPageChunk(opts: { html: string, done: boolean }): MaybePromise<string | undefined>` — applies custom transforms to HTML. If `done` is true, it's the final chunk. Chunks are not guaranteed to be well-formed HTML (they could include an element's opening tag but not its closing tag, for example) but they will always be split at sensible boundaries such as `%sveltekit.head%` or layout/page components.
 - `filterSerializedResponseHeaders(name: string, value: string): boolean` — determines which headers should be included in serialized responses when a `load` function loads a resource with `fetch`. By default, none will be included.
-- `preload(input: { type: 'js' | 'css' | 'font' | 'asset', path: string }): boolean` — determines what files should be added to the `<head>` tag to preload it. The method is called with each file that was found at build time while constructing the code chunks — so if you for example have `import './styles.css` in your `+page.svelte`, `preload` will be called with the resolved path to that CSS file when visiting that page. Preloading can improve performance because things are downloaded sooner, but they can also hurt core web vitals because too many things may be downloaded unnecessarily. By default, `js` and `css` files will be preloaded. `asset` files are not preloaded at all currently, but we may add this later after evaluating feedback.
+- `preload(input: { type: 'js' | 'css' | 'font' | 'asset', path: string }): boolean` — determines what files should be added to the `<head>` tag to preload it. The method is called with each file that was found at build time while constructing the code chunks — so if you for example have `import './styles.css` in your `+page.svelte`, `preload` will be called with the resolved path to that CSS file when visiting that page. Note that in dev mode `preload` is _not_ called, since it depends on analysis that happens at build time. Preloading can improve performance by downloading assets sooner, but it can also hurt if too much is downloaded unnecessarily. By default, `js` and `css` files will be preloaded. `asset` files are not preloaded at all currently, but we may add this later after evaluating feedback.
 
 ```js
 /// file: src/hooks.server.js
diff --git a/packages/kit/CHANGELOG.md b/packages/kit/CHANGELOG.md
index 3ce015167..b0149e9ae 100644
--- a/packages/kit/CHANGELOG.md
+++ b/packages/kit/CHANGELOG.md
@@ -1,5 +1,11 @@
 # @sveltejs/kit
 
+## 1.0.0-next.561
+
+### Patch Changes
+
+- Upgrade vite-plugin-svelte to ^1.3.1 ([#7760](https://github.com/sveltejs/kit/pull/7760))
+
 ## 1.0.0-next.560
 
 ### Patch Changes
diff --git a/packages/kit/package.json b/packages/kit/package.json
index 1edb64e06..1285e6d09 100644
--- a/packages/kit/package.json
+++ b/packages/kit/package.json
@@ -1,6 +1,6 @@
 {
 	"name": "@sveltejs/kit",
-	"version": "1.0.0-next.560",
+	"version": "1.0.0-next.561",
 	"repository": {
 		"type": "git",
 		"url": "https://github.com/sveltejs/kit",
diff --git a/packages/kit/src/runtime/server/data/index.js b/packages/kit/src/runtime/server/data/index.js
index 44a0fe296..3b0f7e97c 100644
--- a/packages/kit/src/runtime/server/data/index.js
+++ b/packages/kit/src/runtime/server/data/index.js
@@ -117,12 +117,7 @@ export async function render_data(event, route, options, state, trailing_slash)
 		const error = normalize_error(e);
 
 		if (error instanceof Redirect) {
-			return json_response(
-				JSON.stringify({
-					type: 'redirect',
-					location: error.location
-				})
-			);
+			return redirect_json_response(error);
 		} else {
 			// TODO make it clearer that this was an unexpected error
 			return json_response(JSON.stringify(handle_error_and_jsonify(event, options, error)));
@@ -143,3 +138,15 @@ function json_response(json, status = 200) {
 		}
 	});
 }
+
+/**
+ * @param {Redirect} redirect
+ */
+export function redirect_json_response(redirect) {
+	return json_response(
+		JSON.stringify({
+			type: 'redirect',
+			location: redirect.location
+		})
+	);
+}
diff --git a/packages/kit/src/runtime/server/index.js b/packages/kit/src/runtime/server/index.js
index f5554c4c6..4b9119258 100644
--- a/packages/kit/src/runtime/server/index.js
+++ b/packages/kit/src/runtime/server/index.js
@@ -13,7 +13,7 @@ import {
 	strip_data_suffix
 } from '../../utils/url.js';
 import { exec } from '../../utils/routing.js';
-import { INVALIDATED_HEADER, render_data } from './data/index.js';
+import { INVALIDATED_HEADER, redirect_json_response, render_data } from './data/index.js';
 import { add_cookies_to_headers, get_cookies } from './cookie.js';
 import { create_fetch } from './fetch.js';
 import { Redirect } from '../control.js';
@@ -289,10 +289,23 @@ export async function respond(request, options, state) {
 			}
 		}
 
+		// Edge case: If user does `return Response(30x)` in handle hook while processing a data request,
+		// we need to transform the redirect response to a corresponding JSON response.
+		if (is_data_request && response.status >= 300 && response.status <= 308) {
+			const location = response.headers.get('location');
+			if (location) {
+				return redirect_json_response(new Redirect(/** @type {any} */ (response.status), location));
+			}
+		}
+
 		return response;
 	} catch (error) {
 		if (error instanceof Redirect) {
-			return redirect_response(error.status, error.location);
+			if (is_data_request) {
+				return redirect_json_response(error);
+			} else {
+				return redirect_response(error.status, error.location);
+			}
 		}
 		return handle_fatal_error(event, options, error);
 	}


Test patch:
diff --git a/packages/kit/test/apps/basics/src/hooks.server.js b/packages/kit/test/apps/basics/src/hooks.server.js
index d7541ef1d..2e5938aa5 100644
--- a/packages/kit/test/apps/basics/src/hooks.server.js
+++ b/packages/kit/test/apps/basics/src/hooks.server.js
@@ -1,7 +1,7 @@
 import fs from 'fs';
 import { sequence } from '@sveltejs/kit/hooks';
 import { HttpError } from '../../../../src/runtime/control';
-import { error } from '@sveltejs/kit';
+import { error, redirect } from '@sveltejs/kit';
 
 /**
  * Transform an error into a POJO, by copying its `name`, `message`
@@ -80,6 +80,17 @@ export const handle = sequence(
 		} catch {}
 
 		return response;
+	},
+	async ({ event, resolve }) => {
+		if (event.url.pathname.includes('/redirect/in-handle')) {
+			if (event.url.search === '?throw') {
+				throw redirect(307, event.url.origin + '/redirect/c');
+			} else {
+				return new Response(undefined, { status: 307, headers: { location: '/redirect/c' } });
+			}
+		}
+
+		return resolve(event);
 	}
 );
 
diff --git a/packages/kit/test/apps/basics/src/routes/redirect/+page.svelte b/packages/kit/test/apps/basics/src/routes/redirect/+page.svelte
index 70f0a624f..852940e4e 100644
--- a/packages/kit/test/apps/basics/src/routes/redirect/+page.svelte
+++ b/packages/kit/test/apps/basics/src/routes/redirect/+page.svelte
@@ -7,3 +7,6 @@
 
 <a href="/redirect/missing-status/a">a (missing-status)</a>
 <a href="/redirect/missing-status/b">b (missing-status)</a>
+
+<a href="/redirect/in-handle?throw">in-handle (throw redirect)</a>
+<a href="/redirect/in-handle?response">in-handle (return Response)</a>
diff --git a/packages/kit/test/apps/basics/src/routes/redirect/in-handle/+page.server.js b/packages/kit/test/apps/basics/src/routes/redirect/in-handle/+page.server.js
new file mode 100644
index 000000000..23addb029
--- /dev/null
+++ b/packages/kit/test/apps/basics/src/routes/redirect/in-handle/+page.server.js
@@ -0,0 +1,1 @@
+export function load() {}
diff --git a/packages/kit/test/apps/basics/src/routes/redirect/in-handle/+page.svelte b/packages/kit/test/apps/basics/src/routes/redirect/in-handle/+page.svelte
new file mode 100644
index 000000000..6a72cbc95
--- /dev/null
+++ b/packages/kit/test/apps/basics/src/routes/redirect/in-handle/+page.svelte
@@ -0,0 +1,1 @@
+<h1>in-handle</h1>
diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js
index 8678d1b1a..633ea3b7a 100644
--- a/packages/kit/test/apps/basics/test/test.js
+++ b/packages/kit/test/apps/basics/test/test.js
@@ -1337,6 +1337,32 @@ test.describe('Redirects', () => {
 			expect(await page.textContent('h1')).toBe('Hazaa!');
 		}
 	});
+
+	test('redirect response in handle hook', async ({ baseURL, clicknav, page }) => {
+		await page.goto('/redirect');
+
+		await clicknav('[href="/redirect/in-handle?response"]');
+
+		await page.waitForURL('/redirect/c');
+		expect(await page.textContent('h1')).toBe('c');
+		expect(page.url()).toBe(`${baseURL}/redirect/c`);
+
+		await page.goBack();
+		expect(page.url()).toBe(`${baseURL}/redirect`);
+	});
+
+	test('throw redirect in handle hook', async ({ baseURL, clicknav, page }) => {
+		await page.goto('/redirect');
+
+		await clicknav('[href="/redirect/in-handle?throw"]');
+
+		await page.waitForURL('/redirect/c');
+		expect(await page.textContent('h1')).toBe('c');
+		expect(page.url()).toBe(`${baseURL}/redirect/c`);
+
+		await page.goBack();
+		expect(page.url()).toBe(`${baseURL}/redirect`);
+	});
 });
 
 test.describe('Routing', () => {