-
Notifications
You must be signed in to change notification settings - Fork 23
fix: Improve typings for errors #213
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
Changes from all commits
fbf11ad
cb9ad45
f8c60db
6f00850
bc33df6
8c0994c
6bc2d74
79b7eb8
6b72e98
6edf6ef
7fe1f88
1f3999d
f458e6e
751ce10
fef71d1
2cfb7b8
64f0012
acb968f
be91456
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -71,10 +71,6 @@ | |
| 'c2pa_reader_remote_url', | ||
| ] | ||
|
|
||
| # TODO Bindings: | ||
| # c2pa_reader_is_embedded | ||
| # c2pa_reader_remote_url | ||
|
|
||
|
|
||
| def _validate_library_exports(lib): | ||
| """Validate that all required functions are present in the loaded library. | ||
|
|
@@ -537,71 +533,118 @@ def _setup_function(func, argtypes, restype=None): | |
|
|
||
|
|
||
| class C2paError(Exception): | ||
| """Exception raised for C2PA errors.""" | ||
| """Exception raised for C2PA errors. | ||
|
|
||
| This is the base class for all C2PA exceptions. Catching C2paError will | ||
| catch all typed C2PA exceptions (e.g., C2paError.ManifestNotFound). | ||
| """ | ||
|
|
||
| def __init__(self, message: str = ""): | ||
| self.message = message | ||
| super().__init__(message) | ||
|
|
||
| class Assertion(Exception): | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Specific exceptions (all the types nested in the parent class) got un-nested from here so they can inherit from the base C2pa exception and get the C2paException type. |
||
| """Exception raised for assertion errors.""" | ||
| pass | ||
|
|
||
| class AssertionNotFound(Exception): | ||
| """Exception raised when an assertion is not found.""" | ||
| pass | ||
| # Define typed exception subclasses that inherit from C2paError | ||
| # These are attached to C2paError as class attributes for backward compatibility | ||
| # (eg., C2paError.ManifestNotFound), and also to ensure properly inheritance hierarchy | ||
|
|
||
| class _C2paAssertion(C2paError): | ||
| """Exception raised for assertion errors.""" | ||
| pass | ||
|
|
||
|
|
||
| class _C2paAssertionNotFound(C2paError): | ||
| """Exception raised when an assertion is not found.""" | ||
| pass | ||
|
|
||
|
|
||
| class _C2paDecoding(C2paError): | ||
| """Exception raised for decoding errors.""" | ||
| pass | ||
|
|
||
|
|
||
| class _C2paEncoding(C2paError): | ||
| """Exception raised for encoding errors.""" | ||
| pass | ||
|
|
||
|
|
||
| class _C2paFileNotFound(C2paError): | ||
| """Exception raised when a file is not found.""" | ||
| pass | ||
|
|
||
|
|
||
| class _C2paIo(C2paError): | ||
| """Exception raised for IO errors.""" | ||
| pass | ||
|
|
||
|
|
||
| class _C2paJson(C2paError): | ||
| """Exception raised for JSON errors.""" | ||
| pass | ||
|
|
||
|
|
||
| class _C2paManifest(C2paError): | ||
| """Exception raised for manifest errors.""" | ||
| pass | ||
|
|
||
|
|
||
| class _C2paManifestNotFound(C2paError): | ||
| """ | ||
| Exception raised when a manifest is not found, | ||
| aka there is no C2PA metadata to read | ||
| aka there is no JUMBF data to read. | ||
| """ | ||
| pass | ||
|
|
||
|
|
||
| class Decoding(Exception): | ||
| """Exception raised for decoding errors.""" | ||
| pass | ||
| class _C2paNotSupported(C2paError): | ||
| """Exception raised for unsupported operations.""" | ||
| pass | ||
|
|
||
| class Encoding(Exception): | ||
| """Exception raised for encoding errors.""" | ||
| pass | ||
|
|
||
| class FileNotFound(Exception): | ||
| """Exception raised when a file is not found.""" | ||
| pass | ||
| class _C2paOther(C2paError): | ||
| """Exception raised for other errors.""" | ||
| pass | ||
|
|
||
| class Io(Exception): | ||
| """Exception raised for IO errors.""" | ||
| pass | ||
|
|
||
| class Json(Exception): | ||
| """Exception raised for JSON errors.""" | ||
| pass | ||
| class _C2paRemoteManifest(C2paError): | ||
| """Exception raised for remote manifest errors.""" | ||
| pass | ||
|
|
||
| class Manifest(Exception): | ||
| """Exception raised for manifest errors.""" | ||
| pass | ||
|
|
||
| class ManifestNotFound(Exception): | ||
| """Exception raised when a manifest is not found.""" | ||
| pass | ||
| class _C2paResourceNotFound(C2paError): | ||
| """Exception raised when a resource is not found.""" | ||
| pass | ||
|
|
||
| class NotSupported(Exception): | ||
| """Exception raised for unsupported operations.""" | ||
| pass | ||
|
|
||
| class Other(Exception): | ||
| """Exception raised for other errors.""" | ||
| pass | ||
| class _C2paSignature(C2paError): | ||
| """Exception raised for signature errors.""" | ||
| pass | ||
|
|
||
| class RemoteManifest(Exception): | ||
| """Exception raised for remote manifest errors.""" | ||
| pass | ||
|
|
||
| class ResourceNotFound(Exception): | ||
| """Exception raised when a resource is not found.""" | ||
| pass | ||
| class _C2paVerify(C2paError): | ||
| """Exception raised for verification errors.""" | ||
| pass | ||
|
|
||
| class Signature(Exception): | ||
| """Exception raised for signature errors.""" | ||
| pass | ||
|
|
||
| class Verify(Exception): | ||
| """Exception raised for verification errors.""" | ||
| pass | ||
| # Attach exception subclasses to C2paError for backward compatibility | ||
| # Preserves behavior for exception catching like except C2paError.ManifestNotFound, | ||
| # also reduces imports (think of it as an alias of sorts) | ||
| C2paError.Assertion = _C2paAssertion | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An we still can use the parent type for general error catching! |
||
| C2paError.AssertionNotFound = _C2paAssertionNotFound | ||
| C2paError.Decoding = _C2paDecoding | ||
| C2paError.Encoding = _C2paEncoding | ||
| C2paError.FileNotFound = _C2paFileNotFound | ||
| C2paError.Io = _C2paIo | ||
| C2paError.Json = _C2paJson | ||
| C2paError.Manifest = _C2paManifest | ||
| C2paError.ManifestNotFound = _C2paManifestNotFound | ||
| C2paError.NotSupported = _C2paNotSupported | ||
| C2paError.Other = _C2paOther | ||
| C2paError.RemoteManifest = _C2paRemoteManifest | ||
| C2paError.ResourceNotFound = _C2paResourceNotFound | ||
| C2paError.Signature = _C2paSignature | ||
| C2paError.Verify = _C2paVerify | ||
|
|
||
|
|
||
| class _StringContainer: | ||
|
|
@@ -656,60 +699,106 @@ def _convert_to_py_string(value) -> str: | |
| return py_string | ||
|
|
||
|
|
||
| def _raise_typed_c2pa_error(error_str: str) -> None: | ||
| """Parse an error string and raise the appropriate typed C2paError. | ||
|
|
||
| Error strings from the native library have the format "ErrorType: message". | ||
| This function parses the error type and raises the corresponding | ||
| C2paError subclass with the full original error string as the message. | ||
|
|
||
| Args: | ||
| error_str: The error string from the native library | ||
|
|
||
| Raises: | ||
| C2paError subclass: The appropriate typed exception based on error_str | ||
| """ | ||
| # Error format from native library is "ErrorType: message" or "ErrorType message" | ||
| # Try splitting on ": " first (colon-space), then fall back to space only | ||
| if ': ' in error_str: | ||
| parts = error_str.split(': ', 1) | ||
| else: | ||
| parts = error_str.split(' ', 1) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the c api is inconsistent here, we should add an issue to fix it so that we always use the colon |
||
| if len(parts) > 1: | ||
| error_type = parts[0] | ||
| # Use the full error string as the message for backward compatibility | ||
| if error_type == "Assertion": | ||
| raise C2paError.Assertion(error_str) | ||
| elif error_type == "AssertionNotFound": | ||
| raise C2paError.AssertionNotFound(error_str) | ||
| elif error_type == "Decoding": | ||
| raise C2paError.Decoding(error_str) | ||
| elif error_type == "Encoding": | ||
| raise C2paError.Encoding(error_str) | ||
| elif error_type == "FileNotFound": | ||
| raise C2paError.FileNotFound(error_str) | ||
| elif error_type == "Io": | ||
| raise C2paError.Io(error_str) | ||
| elif error_type == "Json": | ||
| raise C2paError.Json(error_str) | ||
| elif error_type == "Manifest": | ||
| raise C2paError.Manifest(error_str) | ||
| elif error_type == "ManifestNotFound": | ||
| raise C2paError.ManifestNotFound(error_str) | ||
| elif error_type == "NotSupported": | ||
| raise C2paError.NotSupported(error_str) | ||
| elif error_type == "Other": | ||
| raise C2paError.Other(error_str) | ||
| elif error_type == "RemoteManifest": | ||
| raise C2paError.RemoteManifest(error_str) | ||
| elif error_type == "ResourceNotFound": | ||
| raise C2paError.ResourceNotFound(error_str) | ||
| elif error_type == "Signature": | ||
| raise C2paError.Signature(error_str) | ||
| elif error_type == "Verify": | ||
| raise C2paError.Verify(error_str) | ||
| # If no recognized error type, raise base C2paError | ||
| raise C2paError(error_str) | ||
|
|
||
|
|
||
| def _parse_operation_result_for_error( | ||
| result: ctypes.c_void_p | None, | ||
| check_error: bool = True) -> Optional[str]: | ||
| """Helper function to handle string results from C2PA functions.""" | ||
| """Helper function to handle string results from C2PA functions. | ||
|
|
||
| When result is falsy and check_error is True, this function retrieves the | ||
| error from the native library, parses it, and raises a typed C2paError. | ||
|
|
||
| When result is truthy (a pointer to an error string), this function | ||
| converts it to a Python string, parses it, and raises a typed C2paError. | ||
|
|
||
| Args: | ||
| result: A pointer to a result string, or None/falsy on error | ||
| check_error: Whether to check for errors when result is falsy | ||
|
|
||
| Returns: | ||
| None if no error occurred | ||
|
|
||
| Raises: | ||
| C2paError subclass: The appropriate typed exception if an error occurred | ||
| """ | ||
| if not result: # pragma: no cover | ||
| if check_error: | ||
| error = _lib.c2pa_error() | ||
| if error: | ||
| error_str = ctypes.cast( | ||
| error, ctypes.c_char_p).value.decode('utf-8') | ||
| _lib.c2pa_string_free(error) | ||
| parts = error_str.split(' ', 1) | ||
| if len(parts) > 1: | ||
| error_type, message = parts | ||
| if error_type == "Assertion": | ||
| raise C2paError.Assertion(message) | ||
| elif error_type == "AssertionNotFound": | ||
| raise C2paError.AssertionNotFound(message) | ||
| elif error_type == "Decoding": | ||
| raise C2paError.Decoding(message) | ||
| elif error_type == "Encoding": | ||
| raise C2paError.Encoding(message) | ||
| elif error_type == "FileNotFound": | ||
| raise C2paError.FileNotFound(message) | ||
| elif error_type == "Io": | ||
| raise C2paError.Io(message) | ||
| elif error_type == "Json": | ||
| raise C2paError.Json(message) | ||
| elif error_type == "Manifest": | ||
| raise C2paError.Manifest(message) | ||
| elif error_type == "ManifestNotFound": | ||
| raise C2paError.ManifestNotFound(message) | ||
| elif error_type == "NotSupported": | ||
| raise C2paError.NotSupported(message) | ||
| elif error_type == "Other": | ||
| raise C2paError.Other(message) | ||
| elif error_type == "RemoteManifest": | ||
| raise C2paError.RemoteManifest(message) | ||
| elif error_type == "ResourceNotFound": | ||
| raise C2paError.ResourceNotFound(message) | ||
| elif error_type == "Signature": | ||
| raise C2paError.Signature(message) | ||
| elif error_type == "Verify": | ||
| raise C2paError.Verify(message) | ||
| return error_str | ||
| _raise_typed_c2pa_error(error_str) | ||
| return None | ||
|
|
||
| # In the case result would be a string already (error message) | ||
| return _convert_to_py_string(result) | ||
| error_str = _convert_to_py_string(result) | ||
| if error_str: | ||
| _raise_typed_c2pa_error(error_str) | ||
| return None | ||
|
|
||
|
|
||
| def sdk_version() -> str: | ||
| """ | ||
| Returns the underlying c2pa-rs/c2pa-c-ffi version string | ||
| c2pa-rs and c2pa-c-ffi versions are in lockstep release, | ||
| so the version string is the same for both and we return | ||
| the shared semantic version number. | ||
| """ | ||
| vstr = version() | ||
| # Example: "c2pa-c/0.60.1 c2pa-rs/0.60.1" | ||
|
|
@@ -721,7 +810,11 @@ def sdk_version() -> str: | |
|
|
||
|
|
||
| def version() -> str: | ||
| """Get the C2PA library version.""" | ||
| """ | ||
| Get the C2PA library version with the fully qualified names | ||
| of the native core libraries (library names and semantic version | ||
| numbers). | ||
| """ | ||
| result = _lib.c2pa_version() | ||
| return _convert_to_py_string(result) | ||
|
|
||
|
|
@@ -2622,7 +2715,7 @@ def set_intent( | |
| - EDIT: Edit of a pre-existing parent asset. | ||
| Must have a parent ingredient. | ||
| - UPDATE: Restricted version of Edit for non-editorial changes. | ||
| Must have only one ingredient as a parent. | ||
| Must have only one ingredient, as a parent. | ||
|
|
||
| Args: | ||
| intent: The builder intent (C2paBuilderIntent enum value) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixing error class hierarchy to leverage typed errors better, especially in the new Reader factory method