Skip to content

Fix TypeError in whisper method when API returns JSON string #26

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 21, 2025

Conversation

johnyrahul
Copy link
Contributor

@johnyrahul johnyrahul commented Jul 21, 2025

What

Fixed a TypeError in the whisper method that occurred when the API returns a JSON string instead of a JSON object.

Why

The existing code assumed that json.loads(response.text) would always return a dictionary, but it can return a string if the response contains a JSON string value. This caused a TypeError: 'str' object does not support item assignment when trying to assign message["status_code"].

How

  • Added robust error handling around json.loads() calls in the whisper method
  • Check if the parsed JSON is a dictionary; if not, wrap it in {"message": str(value)}
  • Handle JSON decode errors by wrapping the raw response text in {"message": response.text}
  • Applied the fix to both error status codes (400, 500, etc.) and 202 (async processing) cases
  • Bumped version to 2.4.2

Relevant Docs

  • LLMWhisperer API documentation
  • Error handling best practices

Related Issues or PRs

Dependencies Versions / Env Variables

  • No new dependencies required
  • Compatible with existing environment

Notes on Testing

Added comprehensive unit tests covering all scenarios:

  • JSON string response with error status codes
  • JSON string response with 202 status
  • Invalid JSON response with error status codes
  • Invalid JSON response with 202 status

All tests pass and verify the fix works correctly.

Screenshots

N/A - This is a backend fix

Checklist

I have read and understood the Contribution Guidelines.

🤖 Generated with Claude Code

- Add robust error handling for JSON string responses in whisper method
- Ensure message is always a dict before assigning status_code
- Handle both JSON decode errors and non-dict JSON responses
- Add comprehensive unit tests for all error scenarios
- Bump version to 2.4.2

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link

filepath function $$\textcolor{#23d18b}{\tt{passed}}$$ SUBTOTAL
$$\textcolor{#23d18b}{\tt{tests/integration/client\_v2\_test.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_usage\_info}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/integration/client\_v2\_test.py}}$$ $$\textcolor{#23d18b}{\tt{test\_whisper\_v2}}$$ $$\textcolor{#23d18b}{\tt{9}}$$ $$\textcolor{#23d18b}{\tt{9}}$$
$$\textcolor{#23d18b}{\tt{tests/integration/client\_v2\_test.py}}$$ $$\textcolor{#23d18b}{\tt{test\_highlight}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/integration/client\_v2\_test.py}}$$ $$\textcolor{#23d18b}{\tt{test\_whisper\_v2\_url\_in\_post}}$$ $$\textcolor{#23d18b}{\tt{4}}$$ $$\textcolor{#23d18b}{\tt{4}}$$
$$\textcolor{#23d18b}{\tt{tests/integration/client\_v2\_test.py}}$$ $$\textcolor{#23d18b}{\tt{test\_webhook}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/unit/client\_v2\_test.py}}$$ $$\textcolor{#23d18b}{\tt{test\_register\_webhook}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/unit/client\_v2\_test.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_webhook\_details}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/unit/client\_v2\_test.py}}$$ $$\textcolor{#23d18b}{\tt{test\_whisper\_json\_string\_response\_error}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/unit/client\_v2\_test.py}}$$ $$\textcolor{#23d18b}{\tt{test\_whisper\_json\_string\_response\_202}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/unit/client\_v2\_test.py}}$$ $$\textcolor{#23d18b}{\tt{test\_whisper\_invalid\_json\_response\_error}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/unit/client\_v2\_test.py}}$$ $$\textcolor{#23d18b}{\tt{test\_whisper\_invalid\_json\_response\_202}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils\_test.py}}$$ $$\textcolor{#23d18b}{\tt{test\_redact\_key\_normal}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils\_test.py}}$$ $$\textcolor{#23d18b}{\tt{test\_redact\_key\_different\_reveal\_length}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils\_test.py}}$$ $$\textcolor{#23d18b}{\tt{test\_redact\_key\_non\_string\_input}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{TOTAL}}$$ $$\textcolor{#23d18b}{\tt{25}}$$ $$\textcolor{#23d18b}{\tt{25}}$$

Copy link
Contributor

@jaseemjaskp jaseemjaskp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error handling improvements are good, but there's a critical bug in the 202 status handling where non-dict responses will cause a KeyError when accessing whisper_hash. The 202 response path requires the whisper_hash field to continue processing, so invalid JSON or non-dict responses should be treated as errors rather than wrapped. Additionally, there's code duplication between error and 202 handling that could be refactored into a helper method.

@jaseemjaskp jaseemjaskp self-requested a review July 21, 2025 11:11
@jaseemjaskp jaseemjaskp merged commit 8bee30c into main Jul 21, 2025
1 check passed
@jaseemjaskp jaseemjaskp deleted the fix/error_handling branch July 21, 2025 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants