Skip to content

fix: prevent conversation sync failures from breaking response delivery - #5059

Closed
iamemilio wants to merge 1 commit into
ogx-ai:mainfrom
iamemilio:fix/sync-response-to-conversation-error-handling
Closed

fix: prevent conversation sync failures from breaking response delivery#5059
iamemilio wants to merge 1 commit into
ogx-ai:mainfrom
iamemilio:fix/sync-response-to-conversation-error-handling

Conversation

@iamemilio

@iamemilio iamemilio commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

Summary

When a response is created with a conversation parameter, _create_streaming_response syncs the input and output items to the conversation after inference completes. Previously, if this sync failed (e.g. the conversation store was temporarily unavailable or add_items raised), the exception propagated through the streaming generator. For non-streaming callers this meant losing the completed response entirely; for streaming callers the response.completed event was never yielded. In both cases the client lost a successfully generated response because of a secondary side-effect failure.

This PR wraps the conversation sync block in a try/except so that sync failures are logged with the full traceback but don't prevent the client from receiving its response. The _sync_response_to_conversation method itself still raises, so callers that invoke it directly retain the original behavior.

Test plan

  • New test_sync_failure_still_yields_completed_response passes with the fix and fails without it
  • Existing test_sync_response_to_conversation_api_error still passes (method itself still raises)
  • All 9 conversation tests pass

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Mar 5, 2026
@iamemilio
iamemilio force-pushed the fix/sync-response-to-conversation-error-handling branch from 203c73e to 499a9fd Compare March 5, 2026 20:45
Wrap the conversation sync block in _create_streaming_response with a
try/except so that failures in _sync_response_to_conversation or
store_conversation_messages are logged but don't prevent the client
from receiving the successfully generated response.

Made-with: Cursor
@iamemilio
iamemilio force-pushed the fix/sync-response-to-conversation-error-handling branch from 499a9fd to d10ed42 Compare March 5, 2026 20:46
@iamemilio
iamemilio marked this pull request as ready for review March 5, 2026 20:51
@iamemilio

Copy link
Copy Markdown
Contributor Author

Closing: swallowing the sync error would cause silent conversation context loss. The original behavior (propagating the exception) is correct — the client needs to know when a conversation write fails.

@iamemilio iamemilio closed this Mar 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant