Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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