Skip to content

[BUG] A failed response_model conversion silently returns the raw text instead of raising #6735

Description

@LHMQ878

Problem

When response_model is set and instructor cannot coerce the model's output, both
streaming handlers swallow the failure and return the raw text. A caller who asked for
a BaseModel-shaped string gets prose, with no exception and no signal that conversion
was attempted and failed.

_handle_streaming_response and _ahandle_streaming_response both run the
InternalInstructor.to_pydantic() / model_dump_json() conversion inside the try
that wraps chunk consumption. The except Exception below it exists to salvage a
partial response from a stream that broke mid-flight, and it does that by returning
full_response whenever there is content to return:

  • lib/crewai/src/crewai/llm.py:1040 — sync conversion, inside the try
  • lib/crewai/src/crewai/llm.py:1093-1115except Exception → returns full_response when full_response.strip()
  • lib/crewai/src/crewai/llm.py:1700 — async conversion, inside the try
  • lib/crewai/src/crewai/llm.py:1730-1752except Exception → returns full_response when full_response

A conversion failure is not a broken stream. The stream completed; the output just
didn't match the schema. Routing it through the salvage path conflates the two.

Reproduction

Replace InternalInstructor with one whose to_pydantic() raises, then call with
response_model set and stream=True. Both paths return the prose:

[sync]   NO RAISE, returned 'The Eiffel Tower is in Paris, which has about 2,100,000 residents.'
[async]  NO RAISE, returned 'The Eiffel Tower is in Paris, which has about 2,100,000 residents.'

The non-streaming paths are worth checking under the same lens — llm.py:1242 and
llm.py:1397 also convert, and whether they are inside a comparable recoverable
handler should be part of the fix rather than assumed.

Why this is filed separately

CodeRabbit raised it on #6734 against the async handler only. I didn't take it there:
that PR exists to remove a sync/async divergence, and re-raising in async alone would
have created a new one — call() returning prose while acall() raised, for the same
failure. The behaviour is identical in both handlers today, so the fix belongs in both,
argued on its own terms.

Suggested direction

Move the conversion out of the try, or let a dedicated exception type past the
except Exception, so schema-conversion failures surface while genuine mid-stream
breakage still salvages a partial response. Either way both handlers should change
together, with a test asserting they agree.

Happy to send a PR if the direction sounds right.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions