Skip to content
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

bug: <svelte:boundary> is not working on SSR #15370

Open
adiguba opened this issue Feb 22, 2025 · 17 comments
Open

bug: <svelte:boundary> is not working on SSR #15370

adiguba opened this issue Feb 22, 2025 · 17 comments

Comments

@adiguba
Copy link
Contributor

adiguba commented Feb 22, 2025

Describe the bug

The <svelte:boundary> do nothing on SSR !

On the REPL we can see that code like this :

<svelte:boundary>
	template
	{#snippet failed()}
		error
	{/snippet}
</svelte:boundary>

Will generate this server code :

	{
		function failed($$payload) {
			$$payload.out += `<!---->error`;
		}

		$$payload.out += `<!---->template`;
	}

The template is written, but there no try/catch and the failed() snippet is never caller when an error occurs in the template !

Actually this will produce an HTTP 500 error :(

I think it should render the failed() snippet instead...

Reproduction

Click on the links "Bad page" and "Good page" to see that the error is handheld on client-site.
Then, when on the "Bad page", click on the "reload to force SSR" button to regenerate the page via SSR... => HTTP 500

https://www.sveltelab.dev/ravzziihmogkq1a

Logs

System Info

Done on sveltelabs with last version

Severity

annoyance

@dummdidumm
Copy link
Member

This works as designed, but it's not mentioned in the docs as I realize now

@adiguba
Copy link
Contributor Author

adiguba commented Feb 22, 2025

But IMHO I think the SSR should generate the same result...
It's incorrect to generate an HTTP 500 in this case.

@paoloricciuti
Copy link
Member

But IMHO I think the SSR should generate the same result... It's incorrect to generate an HTTP 500 in this case.

I had the same thought but @trueadm actually gave a good point: if you stream html you can't undo the html you sent

@adiguba
Copy link
Contributor Author

adiguba commented Feb 22, 2025

Yes... but unless I am mistaken, Svelte does not support streaming html...
And even if it was the case, it would still be necessary to make a special case for this (how to cancel a stream on error?)

@paoloricciuti
Copy link
Member

Yes... but unless I am mistaken, Svelte does not support streaming html...

There are plans to support it (Dominic knows more than I do)...and I guess if you error out stopping the stream is a non issue no?

@paoloricciuti
Copy link
Member

That said I think it would be cool to render the fallback

@adiguba
Copy link
Contributor Author

adiguba commented Feb 22, 2025

I tried quickly and I think it's possible to generate something like that :

    // store the current position :
    const $$pos = $$payload.out.length;

    try {
      // the content of the <svelte:boundary/>
      $$payload.out += `<!---->template`;
    } catch ($$err) {
      // reset the buffer to the previous position
      $$payload.out = $$payload.out.substring(0, $$pos);

      // the failed snippet (if any)
      let failed = function($$payload) {
        $$payload.out += `<!---->error`;
      };
      failed($$payload);
    }

But :

  • I don't know if hydration will work correctly (but an hydration mismatch is better than a HTTP 500).
  • I don't know how to handle {@const} correctly. Problem similar to svelte:boundary doesn't catch errors inside a @const tag #15368. They must be inside the try to catch the error, but also outsite in order to be accessible from failed()

@ShourovRoy
Copy link

ShourovRoy commented Feb 24, 2025

to catch the errors, add +error.svelte in that particular route, the boundary is for client only.

https://svelte.dev/docs/kit/errors

@adiguba
Copy link
Contributor Author

adiguba commented Feb 24, 2025

Yes but +error.svelte is for HTTP errors, and I don't want to render an HTTP error...

I just want to render the page correctly, without the svelte:boundary part when it fail... Like with a try/catch...

@trueadm
Copy link
Contributor

trueadm commented Feb 24, 2025

I think we could allow error boundaries to work, but they'd need to use the failed snippet approach, rather than hand rolling a custom approach that renders the error fallback elsewhere, as there's no state on the server.

@adiguba
Copy link
Contributor Author

adiguba commented Feb 26, 2025

I tried a little POC.

Currently it make a code like that :

$.boundary(
	$$payload,
	($$payload2) => {
		$$payload2.out += `<!---->template`;
	},
	($$payload2) => {
		$$payload2.out += `<!---->error`;
	}
);

With a $.boundary() function a simple as that :

/**
 * <svelte:boundary> for server-side
 * @param {Payload} payload
 * @param {(payload:Payload) => void} body
 * @param {(payload:Payload, err: any) => void} [failed]
 * @returns {void}
 */
export function boundary(payload, body, failed) {
	var inner_payload = copy_payload(payload);
	try {
		inner_payload.out += BLOCK_OPEN;
		body(inner_payload);
	} catch (err) {
		inner_payload = copy_payload(payload);
		inner_payload.out += BLOCK_OPEN_ELSE;
		failed?.(inner_payload, err);
	}
	inner_payload.out += BLOCK_CLOSE;
	assign_payload(payload, inner_payload);
}

The hydration in client still need some works, but it seems doable to me...

But the big problem comes from {@const} used on the failed snippet.
I don't known how to handle it correctly... and especially in the case where one of them generates an exception !

@trueadm
Copy link
Contributor

trueadm commented Feb 26, 2025

But the big problem comes from {@const} used on the failed snippet. I don't known how to handle it correctly... and especially in the case where one of them generates an exception !

Yeah that's the biggest issue here. It's what happens when we need to re-bubble up an error. For these reasons, I suggest we keep it client side only until we either a) provide a different implementation that doesn't impact SSR perf, b) add another design or recommendation for error handling during SSR and ensure it works nicely with SvelteKit.

@adiguba
Copy link
Contributor Author

adiguba commented Feb 26, 2025

Perhaps we should forbid using {@const} declared into a <svelte:boundary> from the failed snippet.
(personally I don't find this very nice)

@paoloricciuti
Copy link
Member

Perhaps we should forbid using {@const} declared into a <svelte:boundary> from the failed snippet. (personally I don't find this very nice)

This would also be a breaking change no?

@trueadm
Copy link
Contributor

trueadm commented Feb 26, 2025

This would also be a breaking change no?

Yep, it would be. So we can't do that.

@adiguba
Copy link
Contributor Author

adiguba commented Feb 26, 2025

a) provide a different implementation that doesn't impact SSR perf,

What about a try/catch markup ?

{#try}
   <div>template</div>
{:catch err}
   <pre>error {err}</pre>
{/try}

@paoloricciuti
Copy link
Member

a) provide a different implementation that doesn't impact SSR perf,

What about a try/catch markup ?

{#try}

template
{:catch err}
error {err}
{/try}

We did explore this but svelte:boundary is just better on multiple levels and it will be used for async stuff too so that's a no go too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants