-
Notifications
You must be signed in to change notification settings - Fork 120
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,9 @@ | ||
| <script lang="ts"> | ||
| import { onMount, type Snippet } from 'svelte'; | ||
| import { type Snippet } from 'svelte'; | ||
| import { page } from '$app/stores'; | ||
| import { page } from '$app/state'; | ||
| import Button from '$lib/holocene/button.svelte'; | ||
| import { authUser } from '$lib/stores/auth-user'; | ||
| import type { Memo } from '$lib/types'; | ||
| import type { EventAttribute, WorkflowEvent } from '$lib/types/events'; | ||
|
|
@@ -23,33 +24,41 @@ | |
| key?: string; | ||
| onDecode?: (decodedValue: string) => void; | ||
| children: Snippet<[decodedValue: string]>; | ||
| error?: Snippet<[retry: () => Promise<string>, err: unknown]>; | ||
| loading?: Snippet<[keyedVal: string]>; | ||
| } | ||
| let { children, value, key = '', onDecode }: Props = $props(); | ||
| let { | ||
| children, | ||
| value, | ||
| key = '', | ||
| onDecode, | ||
| error = errorSnip, | ||
| loading = loadingSnip, | ||
| }: Props = $props(); | ||
| let keyedValue = key && value?.[key] ? value[key] : value; | ||
| let decodedValue = $state(stringifyWithBigInt(keyedValue)); | ||
| let keyedValue = stringifyWithBigInt( | ||
| key && value?.[key] ? value[key] : value, | ||
| ); | ||
| onMount(() => { | ||
| decodePayloads(value); | ||
| }); | ||
| let decodeValuePromise = $state<Promise<string>>(decodePayloads(value)); | ||
| const decodePayloads = async ( | ||
| async function decodePayloads( | ||
| _value: PotentiallyDecodable | EventAttribute | WorkflowEvent | Memo, | ||
| ) => { | ||
| ) { | ||
| const settings = { | ||
| ...$page.data.settings, | ||
| ...page.data.settings, | ||
| codec: { | ||
| ...$page.data.settings?.codec, | ||
| endpoint: getCodecEndpoint($page.data.settings), | ||
| passAccessToken: getCodecPassAccessToken($page.data.settings), | ||
| includeCredentials: getCodecIncludeCredentials($page.data.settings), | ||
| ...page.data.settings?.codec, | ||
| endpoint: getCodecEndpoint(page.data.settings), | ||
| passAccessToken: getCodecPassAccessToken(page.data.settings), | ||
| includeCredentials: getCodecIncludeCredentials(page.data.settings), | ||
| }, | ||
| }; | ||
| try { | ||
| const convertedAttributes = await cloneAllPotentialPayloadsWithCodec( | ||
| _value, | ||
| $page.params.namespace, | ||
| page.params.namespace, | ||
| settings, | ||
| $authUser.accessToken, | ||
| ); | ||
|
|
@@ -61,14 +70,36 @@ | |
| if (Array.isArray(finalValue) && finalValue.length === 1) { | ||
| finalValue = finalValue[0]; | ||
| } | ||
| decodedValue = stringifyWithBigInt(finalValue); | ||
| let decodedValue = stringifyWithBigInt(finalValue); | ||
| if (onDecode) { | ||
| onDecode(decodedValue); | ||
| } | ||
| return decodedValue; | ||
| } catch (e) { | ||
| console.error('Could not decode payloads'); | ||
| // hmm before this just ate the error we want to throw this to get an error here | ||
| // 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 | ||
| throw e; | ||
| } | ||
| }; | ||
| } | ||
| </script> | ||
|
|
||
| {@render children(decodedValue)} | ||
| {#snippet loadingSnip(val)} | ||
| {val} | ||
| {/snippet} | ||
|
|
||
| {#snippet errorSnip(retry, error)} | ||
| <div>{error}</div> | ||
|
|
||
| <Button on:click={retry}>Retry Decoding</Button> | ||
| {/snippet} | ||
|
|
||
| {#await decodeValuePromise} | ||
| {@render loading?.(keyedValue)} | ||
| {:then decoded} | ||
| {@render children(decoded)} | ||
| {:catch err} | ||
| {@render error?.(() => decodePayloads(value), err)} | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This... confuses me? So you'd pass something like
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. retry is a function so you can call it in the error component if you like should have probably explicitly set the promise above to the decode payloads so it would retry |
||
| {/await} | ||
|
Comment on lines
+99
to
+105
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. Claude finished @Alex-Tideman's task ββ View job Explanation of line 105The code at line 105 (actually the entire block from lines 99-105) is using Svelte 5's The Pattern: {#await decodeValuePromise}
{@render loading?.(keyedValue)}
{:then decoded}
{@render children(decoded)}
{:catch err}
{@render error?.(() => decodePayloads(value), err)}
{/await}Breaking it down:
Why pass a function instead of calling it? The key insight is that line 104 passes {#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 |
||
Uh oh!
There was an error while loading. Please reload this page.
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