sveltejs / svelte

web development for the rest of us
https://svelte.dev
MIT License
80.21k stars 4.27k forks source link

state_unsafe_mutation when an error happened inside the ":then" block #14186

Closed MartkCz closed 3 weeks ago

MartkCz commented 3 weeks ago

Describe the bug

It seems like Svelte gets stuck in an effect state.

Reproduction

https://svelte.dev/playground/6fbbf69c47894154bfa37c9fb83e7ac0?version=5.1.11

Type something in the text input

Logs

No response

System Info

System:
    OS: Linux 5.15 Manjaro Linux
    CPU: (12) x64 AMD Ryzen 5 5600G with Radeon Graphics
    Memory: 10.44 GB / 30.70 GB
    Container: Yes
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 22.6.0 - /usr/bin/node
    npm: 10.8.2 - /usr/bin/npm
    bun: 1.1.24 - /usr/bin/bun
  Browsers:
    Brave Browser: 128.1.69.160
  npmPackages:
    svelte: ^5.1.11 => 5.1.11

Severity

annoyance

trueadm commented 3 weeks ago

What is the {xxx.xxx} all about? It's likely that the error is causing the problem you're seeing.

Conduitry commented 3 weeks ago

I think that's intentional - the report is that Svelte doesn't recover well from runtime errors like this. Which I don't think there's a lot that can be done about until we implement error boundaries, correct?

MartkCz commented 3 weeks ago

It's just to simulate an error. I can change it to:

<script>
    let { need } = $$props;

    let promise = fetch('https://jsonplaceholder.typicode.com/todos/1');
</script>

{#await promise}
{:then value}
    {value.obj.value}
{:catch error}
    {error}
{/await}

same result. It breaks all components on the website, which isn't ideal for production.

paoloricciuti commented 3 weeks ago

@trueadm the problem is that after the first error that happens during rendering of the then block every change is throwing state_unsafe_mutation.

This is because of point number 1 here:

  1. inside the await block if we have restore true we set the active reaction, then we run the block effect for the then function and then we restore back...but if the then function throws we never restore it back (i already fixed this with a try finally
  2. However if you do <input oninput={(e)=>term = e.target.value} value={term} /> instead of binding value the subsequent errors are not there so i suspect there's also something funky going on with bind value
paoloricciuti commented 3 weeks ago

@trueadm the problem is that after the first error that happens during rendering of the then block every change is throwing state_unsafe_mutation.

This is because of point number 1 here:

  1. inside the await block if we have restore true we set the active reaction, then we run the block effect for the then function and then we restore back...but if the then function throws we never restore it back (i already fixed this with a try finally
  2. However if you do <input oninput={(e)=>term = e.target.value} value={term} /> instead of binding value the subsequent errors are not there so i suspect there's also something funky going on with bind value

Ohhh this is because bind:value it's adding an event listener normally so it's not deferred.

trueadm commented 3 weeks ago

Nice find!

paoloricciuti commented 3 weeks ago

Nice find!

Should we change bind:value too? I think it's fine if it adds the listener normally right?

trueadm commented 3 weeks ago

it just needs to use on internally, no?

paoloricciuti commented 3 weeks ago

it just needs to use on internally, no?

Yeah exactly

trueadm commented 3 weeks ago

@paoloricciuti Are you doing that now or should we create an issue for it?

paoloricciuti commented 3 weeks ago

@paoloricciuti Are you doing that now or should we create an issue for it?

I can do it probably In a bit

trueadm commented 3 weeks ago

I've already got a PR incoming :)