Skip to content

Inconsistent exception handling with multiple event handlers #8403

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
adiguba opened this issue Mar 21, 2023 · 2 comments · Fixed by #11263
Closed

Inconsistent exception handling with multiple event handlers #8403

adiguba opened this issue Mar 21, 2023 · 2 comments · Fixed by #11263
Milestone

Comments

@adiguba
Copy link
Contributor

adiguba commented Mar 21, 2023

Describe the bug

There is a difference in exceptions handling when multiple handlers are added on the same event of an element/component.

Imagine we add 3 handlers on the same element, like this :

    <button on:click={handler_01} on:click={handler_02} on:click={handler_03}> ... 

On a HTML node, if a handler fails for some reason and throw an error, its execution will logically be interrupted, but it has no impact on others handlers.

On a Svelte Component, a handler that throw an error will be interrupted, but will also prevent subsequent handlers from executing.

I think Svelte Components should mimic how native events work.

Reproduction

You can see a reproduction of this bug on this REPL :

https://svelte.dev/repl/a4c8c154000142b695f2a4011184dd12?version=3.57.0

There a 3 counters and 3 handlers that increments theses counters.
But the second handler will fail by throwing an error.

If these handlers are added to a <button>, they will all be executed (and all counter will increment)
If these handlers are added to a Component (via event bubbling (on:click) or createEventDispatcher), the last handler will never be executed (and his counter will not be incremented).

Logs

See reproduction

System Info

Svelte v3.57.0 (in REPL)

Severity

annoyance

@adiguba
Copy link
Contributor Author

adiguba commented Mar 21, 2023

I have a fix for that, but I need some information to complete it.

Actually, the bubbling do dispatch() use a forEach() to invoke all the handlers.
Something like this :

	callbacks.slice().forEach(fn => fn.call(source, event));

So any error will stop the forEach(), and others handlers will not be called.

I think this should be changed to something like this :

	callbacks.slice().forEach(fn => {
		try {
			return fn.call(source, event);
		} catch (err) {
			// report the error
			window.reportError(err);
		}
	});

I'm not sure if we can use window.reportError().
It has a good support, but quite recent : https://caniuse.com/mdn-api_reporterror

Other solution would be to fall back to the equivalent setTimeout() trick :

	// report the error
	if (window.reportError) {
		window.reportError(err);
	} else {
		setTimeout(()=> { throw err; }, 0);
	}

Finally, I want to make a test, but it fail because of the "Uncaught Error", and I don't know how to handle that...

@Rich-Harris
Copy link
Member

Looks like we fixed this in Svelte 5 for classic on:click event handlers (though a test might be worthwhile), but there's some weirdness around modern onclick handlers: demo

@Rich-Harris Rich-Harris added this to the 5.0 milestone Apr 1, 2024
dummdidumm added a commit that referenced this issue Apr 19, 2024
When a delegated event handler throws an error, ensure that delegated event handlers above it don't run.
Strictly speaking this is different from attaching multiple separate event listeners because those above would still run, but it's impossible for us to simulate this behavior here.
Fixes #8403
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment