fix(processing): do not wrap JSONDecodeError in JSON mode so retry catches it; add regression tests for JSON/Validation errors in retry (closes #1856)

Co-Authored-By: Jason Liu <[email protected]>

Author: devin-ai-integration[bot]

instructor/processing/function_calls.py

@@ -92,7 +92,7 @@ def _validate_model_from_json(
             return cls.model_validate(parsed, context=validation_context, strict=False)
     except json.JSONDecodeError as e:
         logger.debug(f"JSON decode error: {e}")
-        raise ValueError(f"Failed to parse JSON: {e}") from e
+        raise
     except Exception as e:
         logger.debug(f"Model validation error: {e}")
         raise

tests/test_json_extraction.py

@@ -267,11 +267,19 @@ def test_validate_model_non_strict(self):
         assert result.skills == ["testing"]
 
     def test_validate_model_json_error(self):
-        """Test handling JSON decode errors."""
+        """Test handling JSON decode errors.
+
+        In strict mode, Pydantic raises ValidationError with an 'Invalid JSON' message.
+        """
         invalid_json = '{"name": "Invalid, "age": 20}'  # Missing quote
 
         with pytest.raises(Exception) as excinfo:
             _validate_model_from_json(Person, invalid_json, None, True)
-
-        # Pydantic directly raises validation errors now, not our custom message
         assert "Invalid JSON" in str(excinfo.value)
+
+    def test_validate_model_json_error_non_strict(self):
+        """In non-strict mode, json.loads should raise JSONDecodeError (not wrapped)."""
+        invalid_json = '{"name": "Invalid, "age": 20}'  # Missing quote
+
+        with pytest.raises(json.JSONDecodeError):
+            _validate_model_from_json(Person, invalid_json, None, False)

tests/test_retry_json_mode.py

@@ -0,0 +1,95 @@
+"""
+Test that retry mechanism works correctly with JSON mode.
+Specifically tests that JSONDecodeError is properly caught by retry handler.
+
+This is a regression test for issue #1856.
+"""
+
+import json
+import pytest
+from unittest.mock import Mock, patch
+from pydantic import BaseModel, ValidationError
+
+import instructor
+from instructor.core.exceptions import InstructorRetryException
+from instructor.mode import Mode
+
+
+class User(BaseModel):
+    name: str
+    age: int
+
+
+def test_json_decode_error_caught_by_retry():
+    """Test that JSON errors are caught by retry handler, not generic Exception handler.
+
+    This is a regression test for issue #1856 where JSONDecodeError was wrapped
+    in ValueError, causing it to be caught by the generic Exception handler instead
+    of the specific validation error handler that calls handle_reask_kwargs.
+
+    Note: In strict mode, Pydantic raises ValidationError with 'Invalid JSON' message.
+    In non-strict mode, json.loads raises JSONDecodeError directly.
+    Both are now properly caught by the retry handler.
+    """
+    mock_response = Mock()
+    mock_response.choices = [Mock()]
+    mock_response.choices[0].message = Mock()
+    mock_response.choices[0].message.content = "invalid json {"
+    mock_response.choices[0].finish_reason = "stop"
+    mock_response.usage = None
+
+    mock_client = Mock()
+    mock_client.chat = Mock()
+    mock_client.chat.completions = Mock()
+    mock_client.chat.completions.create = Mock(return_value=mock_response)
+
+    client = instructor.patch(mock_client, mode=Mode.JSON)
+
+    with pytest.raises(InstructorRetryException) as exc_info:
+        client.chat.completions.create(
+            model="gpt-4o-mini",
+            response_model=User,
+            messages=[{"role": "user", "content": "test"}],
+            max_retries=2,
+        )
+
+    exception = exc_info.value
+    assert exception.n_attempts == 2
+    assert len(exception.failed_attempts) == 2
+
+    for attempt in exception.failed_attempts:
+        assert isinstance(attempt.exception, (json.JSONDecodeError, ValidationError))
+        if isinstance(attempt.exception, ValidationError):
+            assert "Invalid JSON" in str(attempt.exception)
+
+
+def test_validation_error_caught_by_retry():
+    """Test that ValidationError is still caught by retry handler."""
+    mock_response = Mock()
+    mock_response.choices = [Mock()]
+    mock_response.choices[0].message = Mock()
+    mock_response.choices[0].message.content = '{"name": "John"}'
+    mock_response.choices[0].finish_reason = "stop"
+    mock_response.usage = None
+
+    mock_client = Mock()
+    mock_client.chat = Mock()
+    mock_client.chat.completions = Mock()
+    mock_client.chat.completions.create = Mock(return_value=mock_response)
+
+    client = instructor.patch(mock_client, mode=Mode.JSON)
+
+    with pytest.raises(InstructorRetryException) as exc_info:
+        client.chat.completions.create(
+            model="gpt-4o-mini",
+            response_model=User,
+            messages=[{"role": "user", "content": "test"}],
+            max_retries=2,
+        )
+
+    exception = exc_info.value
+    assert exception.n_attempts == 2
+    assert len(exception.failed_attempts) == 2
+
+    for attempt in exception.failed_attempts:
+        assert isinstance(attempt.exception, ValidationError)