-
Notifications
You must be signed in to change notification settings - Fork 118
Payload retries #2999
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
base: main
Are you sure you want to change the base?
Payload retries #2999
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| // but maybe this is leaking information to the users? But it also might be good | ||
| // to allow that? Think about this harder maybe ask app sec about if this is an okay | ||
| // design choice |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We always want to return the original encoded value if decoding fails
| {:then decoded} | ||
| {@render children(decoded)} | ||
| {:catch err} | ||
| {@render error?.(() => decodePayloads(value), err)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This... confuses me?
So you'd pass something like
{#snippet error(retry, err)}
{#await retry} ...
{:catch}
{err}
{/await}
{/snippet}
| {#await decodeValuePromise} | ||
| {@render loading?.(keyedValue)} | ||
| {:then decoded} | ||
| {@render children(decoded)} | ||
| {:catch err} | ||
| {@render error?.(() => decodePayloads(value), err)} | ||
| {/await} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@claude explain this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Claude finished @Alex-Tideman's task —— View job
Explanation of line 105
The code at line 105 (actually the entire block from lines 99-105) is using Svelte 5's {#await} block syntax to handle the asynchronous decoding operation. Here's what's happening:
The Pattern:
{#await decodeValuePromise}
{@render loading?.(keyedValue)}
{:then decoded}
{@render children(decoded)}
{:catch err}
{@render error?.(() => decodePayloads(value), err)}
{/await}Breaking it down:
-
Line 99-100 (Pending state): While
decodeValuePromiseis still loading, it renders theloadingsnippet, passing the originalkeyedValue(the stringified unencoded value) -
Line 101-102 (Success state): When the promise resolves successfully, it renders the
childrensnippet with thedecodedvalue -
Line 103-104 (Error state): If decoding fails, it renders the
errorsnippet with two parameters:- First param: A retry function
() => decodePayloads(value)- This is an arrow function that, when called, will re-attempt the decode operation - Second param: The error object
err
- First param: A retry function
Why pass a function instead of calling it?
The key insight is that line 104 passes () => decodePayloads(value) (a function) rather than calling it immediately. This gives control to the error snippet to decide when to retry. The default errorSnip (lines 93-97) calls this function when the user clicks the "Retry Decoding" button:
{#snippet errorSnip(retry, error)}
<div>{error}</div>
<Button on:click={retry}>Retry Decoding</Button>
{/snippet}This pattern allows consumers to provide their own error UI with custom retry behavior while keeping the retry logic (calling decodePayloads) encapsulated in the component.
Description & motivation 💭
Screenshots (if applicable) 📸
Design Considerations 🎨
Testing 🧪
How was this tested 👻
Steps for others to test: 🚶🏽♂️🚶🏽♀️
Checklists
Draft Checklist
Merge Checklist
Issue(s) closed
Docs
Any docs updates needed?