Skip to content

Cacahfla/error fixes#330

Closed
cachafla wants to merge 2 commits intomainfrom
cacahfla/error-fixes
Closed

Cacahfla/error fixes#330
cachafla wants to merge 2 commits intomainfrom
cacahfla/error-fixes

Conversation

@cachafla
Copy link
Contributor

@cachafla cachafla commented Mar 7, 2025

Internal Notes for Reviewers

refactor: Update error handling and API error mapping in errors.py

  • Add UnauthorizedError for handling unauthorized API actions
  • Replace InvalidProjectError with InvalidModelIdError
  • Modify raise_api_error() to handle more complex error response structures
  • Update error code mappings to match new API error conventions
  • Remove deprecated error classes and mappings

I noticed we were getting obscure errors after the API errors refactor:

File ~/work/validmind-library/validmind-library/sdist-venv/lib/python3.11/site-packages/ipykernel/zmqshell.py:559, in ZMQInteractiveShell._showtraceback(self, etype, evalue, stb)
    553 sys.stdout.flush()
    554 sys.stderr.flush()
    556 exc_content = {
    557     "traceback": stb,
    558     "ename": str(etype.__name__),
--> 559     "evalue": str(evalue),
    560 }
    562 dh = self.displayhook
    563 # Send exception info over pub socket for other clients than the caller
    564 # to pick up

TypeError: __str__ returned non-string (type NoneType)

Note that we still need to migrate this:

# ValidMind Library (VML) specific errors
VML_MISSING_TEXT = "missing_text"  # Missing text attribute
VML_INVALID_TEXT_OBJECT = "invalid_text_object"  # Invalid text object
VML_INVALID_CONTENT_ID_PREFIX = "invalid_content_id_prefix"  # Invalid content ID prefix
VML_INVALID_TEST_RESULTS = "invalid_test_results"  # Invalid test results

External Release Notes

cachafla added 2 commits March 7, 2025 14:17
- Add `UnauthorizedError` for handling unauthorized API actions
- Replace `InvalidProjectError` with `InvalidModelIdError`
- Modify `raise_api_error()` to handle more complex error response structures
- Update error code mappings to match new API error conventions
- Remove deprecated error classes and mappings
@cachafla cachafla requested a review from jamadriz March 7, 2025 22:19
@github-actions
Copy link
Contributor

github-actions bot commented Mar 7, 2025

PR Summary

This pull request introduces several enhancements to the error handling mechanism within the validmind library. The key changes include:

  1. Introduction of UnauthorizedError: A new error class UnauthorizedError is added, which inherits from APIRequestError. This error is raised when a user is not authorized to perform a specific API action.

  2. Refactoring of Error Handling in raise_api_error: The function raise_api_error has been updated to improve the parsing of error responses. The changes include:

    • Parsing the error and details fields from the JSON response to extract reason and code.
    • Mapping new error codes such as INVALID_MODEL and UNAUTHORIZED to their respective error classes (InvalidModelIdError and UnauthorizedError).
  3. Modification of Error Class: The InvalidProjectError class has been renamed to InvalidModelIdError, and its description has been updated to reflect the change from project ID to model ID.

These changes aim to improve the clarity and accuracy of error reporting within the library, making it easier for developers to diagnose and handle API errors effectively.

Test Suggestions

  • Test UnauthorizedError by simulating an unauthorized API request and verifying that the error is raised correctly.
  • Test the raise_api_error function with various error JSON strings to ensure correct error class mapping and description extraction.
  • Verify that the InvalidModelIdError is raised with the correct description when an invalid model ID is provided.
  • Ensure backward compatibility by testing existing functionality that might be affected by the changes in error handling.

@cachafla cachafla added the internal Not to be externalized in the release notes label Mar 7, 2025
Copy link

@jamadriz jamadriz left a comment

Choose a reason for hiding this comment

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

👍🏼

@cachafla cachafla closed this Mar 18, 2025
@cachafla cachafla deleted the cacahfla/error-fixes branch March 18, 2025 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal Not to be externalized in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants