fix(api): await operations in createConversationWithPlaceholder to pr…

[ErikBjare]

…opagate errors

Changes createConversationWithPlaceholder from fire-and-forget pattern to awaiting operations, allowing errors to propagate to caller's try-catch.

Before: Operations ran in background promises (.then/.catch), errors
only logged, never reaching UI error handling.

After: Operations awaited, errors naturally propagate to WelcomeView.tsx
error handling, showing proper error messages to users.

Benefits:

• Error handling in WelcomeView.tsx now works correctly
• Users see specific error messages (duplicates, network, auth)
• Users won't navigate to failed conversations
• Simpler, more maintainable code

Fixes architectural issue identified by Greptile and Erik. Addresses #37

---

Important

createConversationWithPlaceholder in api.ts now awaits operations to propagate errors to the UI, improving error handling and user experience.

• Behavior:
  • createConversationWithPlaceholder in api.ts now awaits createConversation and step operations instead of using .then/.catch.
  • Errors propagate to WelcomeView.tsx, allowing UI to handle errors like duplicates, network, and auth issues.
• Benefits:
  • Users see specific error messages and won't navigate to failed conversations.
  • Code is simpler and more maintainable.
• Fixes:
  • Addresses architectural issue identified by Greptile and Erik.
  • Fixes Better error handling on duplicate conversation names #37.

^{This description was created by for 3452137. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed everything up to 3452137 in 49 seconds. Click for details.

• Reviewed 41 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 3 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. src/utils/api.ts:634

• Draft comment:
  Good change: awaiting createConversation now propagates errors instead of silently logging them. Ensure that the caller’s try-catch handles these errors.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

2. src/utils/api.ts:643

• Draft comment:
  Awaiting the auto-trigger step ensures any errors in generating the response are propagated. Confirm that blocking navigation on step failure is the intended behavior.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

3. src/utils/api.ts:647

• Draft comment:
  The return now occurs only after successful completion of both operations. Make sure this change aligns with UI navigation expectations in case of errors.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_AdZXRVPE44TucbSn

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[greptile-apps]

Greptile Overview

Greptile Summary

Refactored createConversationWithPlaceholder from fire-and-forget pattern to awaiting operations, allowing errors from createConversation and step to properly propagate to WelcomeView.tsx error handlers.

Key Changes

• Removed .then()/.catch() promise chains in favor of await statements
• Error messages now reach UI error handling in WelcomeView.tsx:47-84
• Users will see specific error messages for duplicates, network issues, and auth failures
• Users won't navigate to failed conversations

Issues Found

• Critical: Missing cleanup logic for placeholder conversations when server operations fail (lines 625-648)
  • If createConversation or step throw errors, the placeholder conversation created on line 625 remains in the store
  • This creates orphaned conversations in the UI that don't exist on the server
  • Needs try-catch wrapper with conversations$.delete(conversationId) in catch block

Confidence Score: 2/5

• This PR improves error propagation but introduces a critical bug that leaves orphaned conversations in the store when server operations fail
• While the change correctly addresses the architectural issue of error propagation (fixing issue #37), it creates a new bug where placeholder conversations are not cleaned up on error. This will cause user-visible issues where failed conversations appear in the UI. The fix is straightforward (add try-catch with cleanup), but the current implementation has a definite bug that needs resolution before merging.
• src/utils/api.ts requires immediate attention - add error handling with cleanup in createConversationWithPlaceholder

Important Files Changed

File Analysis

Filename	Score	Overview
src/utils/api.ts	2/5	Changed from fire-and-forget to awaited operations for proper error propagation, but missing cleanup logic for orphaned placeholder conversations on error

Sequence Diagram

sequenceDiagram
    participant User
    participant WelcomeView
    participant ApiClient
    participant Store
    participant Server

    User->>WelcomeView: Enter message & click send
    WelcomeView->>ApiClient: createConversationWithPlaceholder(message)
    ApiClient->>ApiClient: Generate conversationId
    ApiClient->>Store: initConversation(id, placeholder)
    Note over Store: Placeholder conversation<br/>created in local store
    
    ApiClient->>Server: createConversation(id, message)
    alt Success
        Server-->>ApiClient: {status, session_id}
        ApiClient->>Server: step(id)
        alt Success
            Server-->>ApiClient: Generation started
            ApiClient-->>WelcomeView: conversationId
            WelcomeView->>WelcomeView: navigate(`/chat/${id}`)
            WelcomeView->>User: Show success toast
        else Step fails
            Server-->>ApiClient: Error
            Note over Store: ⚠️ Orphaned conversation<br/>remains in store
            ApiClient-->>WelcomeView: throw error
            WelcomeView->>User: Show error toast
        end
    else Create fails
        Server-->>ApiClient: Error (duplicate/auth/network)
        Note over Store: ⚠️ Orphaned conversation<br/>remains in store
        ApiClient-->>WelcomeView: throw error
        WelcomeView->>User: Show specific error toast
    end

Loading

Additional Comments (1)

1. src/utils/api.ts, line 625-632 (link)

   logic: placeholder conversation in store is not cleaned up on error

   If createConversation or step fail after line 625, the placeholder conversation remains in the store but never gets properly initialized on the server. This orphan conversation could confuse users.

_{1 file reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

logic: missing cleanup in catch block

The function should clean up the placeholder conversation from the store if server operations fail. Without cleanup, users may see orphaned conversations in the UI that don't exist on the server.

Add error handling with cleanup:

    try {
      // Await server-side creation and auto-step to propagate errors properly
      await this.createConversation(conversationId, [message], {
        chat: {
          model: options?.model,
          stream: options?.stream,
          workspace: options?.workspace || '.',
        },
      });
      
      // Auto-trigger generation now that the conversation is ready
      await this.step(conversationId, options?.model, options?.stream);

      // Return ID only after operations complete successfully
      return conversationId;
    } catch (error) {
      // Clean up placeholder conversation on error
      conversations$.delete(conversationId);
      throw error;
    }

Note: Need to import conversations$ from @/stores/conversations

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/utils/api.ts
Line: 635:648

Comment:
**logic:** missing cleanup in catch block

The function should clean up the placeholder conversation from the store if server operations fail. Without cleanup, users may see orphaned conversations in the UI that don't exist on the server.

Add error handling with cleanup:
```typescript
    try {
      // Await server-side creation and auto-step to propagate errors properly
      await this.createConversation(conversationId, [message], {
        chat: {
          model: options?.model,
          stream: options?.stream,
          workspace: options?.workspace || '.',
        },
      });
      
      // Auto-trigger generation now that the conversation is ready
      await this.step(conversationId, options?.model, options?.stream);

      // Return ID only after operations complete successfully
      return conversationId;
    } catch (error) {
      // Clean up placeholder conversation on error
      conversations$.delete(conversationId);
      throw error;
    }
```

Note: Need to import `conversations$` from `@/stores/conversations`

How can I resolve this? If you propose a fix, please make it concise.