[BUG] A failed response_model conversion silently returns the raw text instead of raising
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 58.8k
- Forks
- 8.5k
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 109
Description
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 thetrylib/crewai/src/crewai/llm.py:1093-1115—except Exception→ returnsfull_responsewhenfull_response.strip()lib/crewai/src/crewai/llm.py:1700— async conversion, inside thetrylib/crewai/src/crewai/llm.py:1730-1752—except Exception→ returnsfull_responsewhenfull_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.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in lib/crewai/src/crewai/llm.py at _handle_streaming_response (around lines 1040-1115) and _ahandle_streaming_response (around lines 1700-1752), then inspect the non-streaming conversions around lines 1242 and 1397. Add regression coverage for matching sync and async behavior: conversion failures must raise, while genuine mid-stream failures still salvage partial text.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend-api-design
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 73/100