fix: rethrow error of failed iterable after calling `return()` (#18169)

The fix in #17966 wasn't quite right, because we gotta rethrow in case
the iterator stopped because of an error. Fixes part of the SvelteKit
`query.live` test failure.

---------

Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com>
Co-authored-by: Rich Harris <rich.harris@vercel.com>

.changeset/fresh-stars-grin.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: rethrow error of failed iterable after calling `return()`

packages/svelte/src/internal/client/reactivity/async.js

@@ -213,22 +213,35 @@ export async function* for_await_track_reactivity_loss(iterable) {
 		throw new TypeError('value is not async iterable');
 	}
 
-	/** Whether the completion of the iterator was "normal", meaning it wasn't ended via `break` or a similar method */
-	let normal_completion = false;
+	// eslint-disable-next-line no-useless-assignment
+	let invoke_return = true;
+
 	try {
 		while (true) {
 			const { done, value } = (await track_reactivity_loss(iterator.next()))();
 			if (done) {
-				normal_completion = true;
+				invoke_return = false;
 				break;
 			}
 			var prev = reactivity_loss_tracker;
-			yield value;
+			try {
+				yield value;
+			} catch (e) {
+				set_reactivity_loss_tracker(prev);
+				// If the yield throws, we need to call `return` but not return its value, instead rethrow
+				if (iterator.return !== undefined) {
+					(await track_reactivity_loss(iterator.return()))();
+				}
+				throw e;
+			}
 			set_reactivity_loss_tracker(prev);
 		}
+	} catch (error) {
+		invoke_return = false;
+		throw error;
 	} finally {
-		// If the iterator had an abrupt completion and `return` is defined on the iterator, call it and return the value
-		if (!normal_completion && iterator.return !== undefined) {
+		// If the iterator had an abrupt completion (break) and `return` is defined on the iterator, call it and return the value
+		if (invoke_return && iterator.return !== undefined) {
 			// eslint-disable-next-line no-unsafe-finally
 			return /** @type {TReturn} */ ((await track_reactivity_loss(iterator.return()))().value);
 		}

packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-for-await-throws-1/_config.js

@@ -0,0 +1,24 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+import { normalise_trace_logs } from '../../../helpers.js';
+
+export default test({
+	compileOptions: {
+		dev: true
+	},
+	html: '<p>pending</p>',
+	async test({ assert, target, warnings }) {
+		await tick();
+
+		assert.htmlEqual(
+			target.innerHTML,
+			'<h1>number -> number -> number -> return -> body failed -> ended</h1>'
+		);
+
+		assert.deepEqual(normalise_trace_logs(warnings), [
+			{
+				log: 'Detected reactivity loss when reading `values[1]`. This happens when state is read in an async function after an earlier `await`'
+			}
+		]);
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-for-await-throws-1/main.svelte

@@ -0,0 +1,45 @@
+<script>
+	let values = $state([0, 1, 2]);
+
+	async function get_result() {
+		const logs = [];
+
+		const iterator = {
+			index: 0,
+			async next() {
+				if (this.index > 2) { done: true }
+				return { done: false, value: values[this.index++] };
+			},
+			async return() {
+				logs.push('return');
+			},
+			[Symbol.asyncIterator]() {
+				return this;
+			}
+		};
+
+		try {
+			for await (const value of iterator) {
+				logs.push('number');
+				// Read reactive state after async iterator await.
+				if (values.length === 3 && value === 2) {
+					throw new Error('body failed');
+				}
+			}
+			logs.push('done');
+		} catch (error) {
+			logs.push(error.message);
+		}
+
+		logs.push('ended');
+		return logs.join(' -> ');
+	}
+</script>
+
+<svelte:boundary>
+	<h1>{await get_result()}</h1>
+
+	{#snippet pending()}
+		<p>pending</p>
+	{/snippet}
+</svelte:boundary>

packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-for-await-throws-2/_config.js

@@ -0,0 +1,21 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+import { normalise_trace_logs } from '../../../helpers.js';
+
+export default test({
+	compileOptions: {
+		dev: true
+	},
+	html: '<p>pending</p>',
+	async test({ assert, target, warnings }) {
+		await tick();
+
+		assert.htmlEqual(target.innerHTML, '<h1>number -> number -> next failed -> ended</h1>');
+
+		assert.deepEqual(normalise_trace_logs(warnings), [
+			{
+				log: 'Detected reactivity loss when reading `values[1]`. This happens when state is read in an async function after an earlier `await`'
+			}
+		]);
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-for-await-throws-2/main.svelte

@@ -0,0 +1,43 @@
+<script>
+	let values = $state([0, 1, 2]);
+
+	async function get_result() {
+		const logs = [];
+
+		const iterator = {
+			index: 0,
+			async next() {
+				if (this.index > 1) throw new Error('next failed');
+				return { done: false, value: values[this.index++] };
+			},
+			async return() {
+				logs.push('return');
+			},
+			[Symbol.asyncIterator]() {
+				return this;
+			}
+		};
+
+		try {
+			for await (const value of iterator) {
+				logs.push('number');
+				// Read reactive state after async iterator await.
+				values.length === value;
+			}
+			logs.push('done');
+		} catch (error) {
+			logs.push(error.message);
+		}
+
+		logs.push('ended');
+		return logs.join(' -> ');
+	}
+</script>
+
+<svelte:boundary>
+	<h1>{await get_result()}</h1>
+
+	{#snippet pending()}
+		<p>pending</p>
+	{/snippet}
+</svelte:boundary>