Skip to content

Commit

Permalink
fix: Mitigate Prompt modal race condition
Browse files Browse the repository at this point in the history
Add some checks and await the handling of git prompts.
  • Loading branch information
estib-vega committed Oct 28, 2024
1 parent 7bcb7e9 commit 2406720
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 30 deletions.
45 changes: 31 additions & 14 deletions apps/desktop/src/lib/backend/prompt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,40 @@ type FilterParams = {

export type SystemPromptHandle = {
prompt: string;
respond: (response: string | null) => void;
respond: (response: string | null) => Promise<void>;
};

type PromptHandler = (prompt: SystemPrompt, signal: AbortSignal) => Promise<string | null>;
type FilterEventEntry = [filter: FilterParams, handler: PromptHandler];

/**
* Handle the abort signal for a prompt handler.
*
* This is required to be able to await the handling of a prompt.
*/
async function handleAbortSignal(
signal: AbortSignal,
promptStore: Writable<SystemPromptHandle | undefined>,
errorStore: Writable<any>
): Promise<void> {
const signalHandler = new Promise<void>((resolve) => {
signal.addEventListener('abort', () => {
promptStore.set(undefined);
switch (signal.reason) {
case 'timeout':
errorStore.set('Timed out waiting for response');
break;
default:
errorStore.set(undefined);
break;
}
resolve();
});
});

await signalHandler;
}

/**
* This service is used for handling CLI prompts from the back end.
*/
Expand Down Expand Up @@ -127,20 +155,9 @@ export class PromptService {

promptStore.set({
prompt: prompt.prompt,
respond: (response: string | null) => {
respond: async (response: string | null) => {
resolver(response);
}
});

signal.addEventListener('abort', () => {
promptStore.set(undefined);
switch (signal.reason) {
case 'timeout':
errorStore.set('Timed out waiting for response');
break;
default:
errorStore.set(undefined);
break;
await handleAbortSignal(signal, promptStore, errorStore);
}
});

Expand Down
38 changes: 22 additions & 16 deletions apps/desktop/src/lib/components/PromptModal.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,22 @@
const promptService = getContext(PromptService);
const [prompt, error] = promptService.reactToPrompt({ timeoutMs: 30000 });
let value = '';
let modal: Modal;
let loading = false;
let value = $state<string>('');
let modal = $state<ReturnType<typeof Modal>>();
let loading = $state(false);
$: if ($prompt) {
modal?.show();
}
$: if (!$prompt && !$error) {
modal?.close();
}
$effect(() => {
if ($prompt && modal?.imports.open === false && !loading) {
modal?.show();
}
});
async function submit() {
if (!$prompt) return;
loading = true;
try {
$prompt.respond(value);
await modal?.close();
await $prompt.respond(value);
} catch (err) {
console.error(err);
} finally {
Expand All @@ -35,14 +34,19 @@
async function cancel() {
try {
if ($prompt) $prompt.respond(null);
if ($prompt) await $prompt.respond(null);
} catch (err) {
console.error(err);
} finally {
clear();
}
}
async function handleCancelButton() {
await modal?.close();
await cancel();
}
function clear() {
prompt.set(undefined);
error.set(undefined);
Expand All @@ -53,9 +57,9 @@
<Modal
bind:this={modal}
width="small"
title="Git fetch requires input"
onClose={async () => await cancel()}
onSubmit={async () => await submit()}
title="Git needs input"
onClickOutside={cancel}
onSubmit={submit}
>
<div class="message">
{#if $error}
Expand All @@ -67,7 +71,9 @@
<Textbox autofocus type="password" bind:value disabled={!!$error || loading} />

{#snippet controls()}
<Button style="ghost" type="reset" outline disabled={loading} onclick={cancel}>Cancel</Button>
<Button style="ghost" type="reset" outline disabled={loading} onclick={handleCancelButton}
>Cancel</Button
>
<Button style="pop" type="submit" kind="solid" grow disabled={!!$error || loading} {loading}>
Submit
</Button>
Expand Down

0 comments on commit 2406720

Please sign in to comment.