sites icon indicating copy to clipboard operation
sites copied to clipboard

REPL builder crashes on infinite loops

Open brandonmcconnell opened this issue 2 years ago • 6 comments

Describe the bug

The REPL builder crashes on infinite loops rather than reporting them via an error and allowing the dev to modify/save the REPL to resolve the bug.

Oftentimes, if this happens in a REPL I'm working on, I have to continuously reload the page, quickly switch tabs, and copy the source to a local editor to resolve the bug, only to then paste the code back into a fresh REPL, as the infinite looping one is endlessly stuck.

Reproduction

I've created one such example that shouldn't crash immediately (on larger screens at least). However, if you shrink the viewport size of the REPL preview to where the text gets truncated, you'll see it freezes completely.

REPL: https://svelte.dev/repl/deaa7c99b06f4ecf8921174a88b33538?version=3.50.1

Logs

No response

System Info

Unnecessary — not local issue

Severity

annoyance

brandonmcconnell avatar Sep 21 '22 13:09 brandonmcconnell

This is the wrong repo for REPL, see https://github.com/sveltejs/sites/tree/master/packages/repl

I was about to comment the following on your original issue:

It'd be helpful if the REPL could somehow report that in an error and prevent the editor from crashing

Sounds like the halting problem. The REPL does prevent regular vanilla endless loops by injecting stuff (see loopGuardTimeout option). What you have here is impossible to detect in my opinion. It's an async/"recursiveish" endless loop (the browser will notify the observer in the next tick I would assume). But maybe some trickery is possible, but I don't see anyone invest considerable amounts of time in this.

However, one thing I would like see is the option to disable auto-eval in REPL. This would give us the chance to save before running it. Otherwise you can trigger endless loops just by typing code you didn't mean to run. I usually find it annoying, but I'm sure there's already an issue for that in the repo.

@benmccann can you please move this to the /sites repo?

Prinzhorn avatar Sep 21 '22 13:09 Prinzhorn

Transferred. But I agree that this feature request certainly sounds halting-problem-y.

Conduitry avatar Sep 21 '22 14:09 Conduitry

@Prinzhorn Yeah, if there's some trickery we can do here to detect and prevent that, that'd be great.

I also agree— I think a way to set a REPL to be able to be saved first before running would be ideal, maybe as an option disabled by default.

brandonmcconnell avatar Sep 29 '22 13:09 brandonmcconnell

Btw @Prinzhorn, here is another example that demonstrates the same issue, and I don't quite understand where the recursive issue would be coming from: https://svelte.dev/repl/fa3352f06f004df4b0028f4fc654c8c0?version=3.50.1 ☝🏼 un-comment the {(() => i++)()}

Here is the source, with that line pre-un-commented:

<!-- App.svelte -->

<script>
	let arr = [1, 2, 3];
	let i = 0;
</script>

{#each arr as elem}
	{(() => i++)()}
{/each}

☝🏼 this will break

What is it about running {(() => i++)()} within the template that causes such a break?

brandonmcconnell avatar Sep 29 '22 13:09 brandonmcconnell

& related but without causing the loop issue— why, when I move the i++ logic into the script logic does it computed before the loop so all iterations have the same value: https://svelte.dev/repl/a9c8110a13bb4ff88960850b96b2e2a4?version=3.50.1

Source:

<!-- App.svelte -->

<script>
	let arr = [1, 2, 3];
	let i = 0;
	let skip = (num = 1) => {
		i += num;
		return '';
	};
</script>

{#each arr as elem}
	{skip()} <!-- always renders 3 -->
	{i}
{/each}

brandonmcconnell avatar Sep 29 '22 13:09 brandonmcconnell

Your last two examples are not directly related to this issue here. There are issues in Svelte about this already (https://github.com/sveltejs/svelte/issues/7295#issuecomment-1046235644) and I personally think that your examples should straight up not compile in Svelte 4. I think at least some of the Svelte team was in the same boat as me on that. Mutating state from inside the template is undefined behavior and doesn't make sense. If you call a function it needs to be pure, e.g. sth. like formatDate(date). It should not have any side effects whatsoever.

Prinzhorn avatar Sep 29 '22 14:09 Prinzhorn