Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/two-lizards-poke.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: Don't crash on hydratable serialization failure
28 changes: 12 additions & 16 deletions packages/svelte/src/internal/server/hydratable.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,32 +57,28 @@ function encode(key, value, unresolved) {

entry.serialized = devalue.uneval(entry.value, (value, uneval) => {
if (is_promise(value)) {
// we serialize promises as `"${i}"`, because it's impossible for that string
// to occur 'naturally' (since the quote marks would have to be escaped)
// this placeholder is returned synchronously from `uneval`, which includes it in the
// serialized string. Later (at least one microtask from now), when `p.then` runs, it'll
// be replaced.
const placeholder = `"${uid++}"`;
const p = value
.then((v) => `r(${uneval(v)})`)
.then((v) => {
entry.serialized = entry.serialized.replace(placeholder, `r(${uneval(v)})`);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As always all this recursive promise stuff is brain-bending, but I think we just had indirection here in the first place. There's no reason we need to actually return the string from this promise, we can just replace the placeholders inline.

})
.catch((devalue_error) =>
e.hydratable_serialization_failed(
key,
serialization_stack(entry.stack, devalue_error?.stack)
)
);

// prevent unhandled rejections from crashing the server
p.catch(() => {});

// track which promises are still resolving when render is complete
unresolved?.set(p, key);
p.finally(() => unresolved?.delete(p));

// we serialize promises as `"${i}"`, because it's impossible for that string
// to occur 'naturally' (since the quote marks would have to be escaped)
const placeholder = `"${uid++}"`;

(entry.promises ??= []).push(
p.then((s) => {
entry.serialized = entry.serialized.replace(placeholder, s);
})
);
// prevent unhandled rejections from crashing the server, track which promises are still resolving when render is complete
p.catch(() => {}).finally(() => unresolved?.delete(p));

(entry.promises ??= []).push(p);
return placeholder;
}
});
Expand Down

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did verify that this crashes the test process prior to this PR -- it ends up with that super unhelpful "some test somewhere had a rejected promise" error. So we're all good now, for sure, and this will catch regressions.

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { test } from '../../test';

export default test({
mode: ['async'],
error: 'hydratable_serialization_failed'
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script lang="ts">
import { hydratable } from 'svelte';
hydratable('key', () => new Promise(() => { throw new Error('nope') }));
</script>
Loading