handleDelegationExit : do not clear if an error is being displayed#281066
handleDelegationExit : do not clear if an error is being displayed#281066joshspicer merged 2 commits intomainfrom
Conversation
joshspicer
changed the title
There was a problem hiding this comment.
Pull request overview
This PR enhances the _handleDelegationExit() function to prevent clearing the chat widget when a delegation completes with an error. Previously, the widget would always clear after delegation regardless of the response outcome. Now, it checks if the final response has error details and preserves the widget in error cases, allowing users to see what went wrong.
Key changes:
- Added error detection logic to check
result?.errorDetailsbefore clearing - Renamed helper function from
checkForCompletetocheckIfShouldClearfor clarity - Modified Promise flow to return boolean indicating whether to clear, rather than always clearing
| const result = checkIfShouldClear(); | ||
| if (result) { | ||
| cleanup(); | ||
| resolve(); | ||
| resolve(true); | ||
| } | ||
| }); | ||
| const timeout = setTimeout(() => { | ||
| cleanup(); | ||
| resolve(); | ||
| resolve(false); | ||
| }, 30_000); // 30 second timeout | ||
| const cleanup = () => { | ||
| clearTimeout(timeout); |
There was a problem hiding this comment.
The function could be optimized to exit early when a response completes with an error. Currently, if a response completes with an error, the function will wait the full 30-second timeout before deciding not to clear, even though it already knows the response is complete and shouldn't be cleared. Consider modifying checkIfShouldClear() to return a tri-state value (e.g., 'clear' | 'dontClear' | 'keepWaiting') to distinguish between "response incomplete" and "response complete with error".
| const lastItem = items[items.length - 1]; | ||
| if (lastItem && isResponseVM(lastItem) && lastItem.model && lastItem.isComplete && !lastItem.model.isPendingConfirmation.get()) { | ||
| return true; | ||
| const hasError = Boolean(lastItem.result?.errorDetails); |
There was a problem hiding this comment.
Consider using lastItem.errorDetails instead of lastItem.result?.errorDetails for consistency with the codebase. The errorDetails property is a getter that returns result?.errorDetails, making both equivalent, but using the direct property is more concise and follows the encapsulation pattern.
| const hasError = Boolean(lastItem.result?.errorDetails); | |
| const hasError = Boolean(lastItem.errorDetails); |
No description provided.