starknet_in_rust
starknet_in_rust copied to clipboard
lambdaclass
Improve the error handling in the implementation of the `storage_read` callback of the `NativeSyscallHandler`
The storage_read callback works but it's error handling has an unhandled case and includes dbg! macros which should at most log the error.
https://github.com/lambdaclass/starknet_in_rust/pull/943/files#diff-050b376c364b46aa56077588d77a5939973603c9caf47e452bc0895b5b6cd161R155
Hello man, Sorry if i see This Issue now but I'm texting you a possible solution. For the error handling of the storage_read callback within NativeSyscallHandler, it is crucial to address specific error types, such as FileNotFoundError, return distinct error codes for identification, log detailed information for debugging, handle permission issues, implement fallback mechanisms for critical operations, and document the error-handling strategy for clarity and understanding. Here I'm writing for you an example and you should adapt it based on the specifics of your implementation and the requirements of Starknet Application python Copy code def storage_read(file_path): try: with open(file_path, 'r') as file: data = file.read() # Process the data return data except FileNotFoundError: # Handle file not found error log_error(f"File not found: {file_path}") return None except PermissionError: # Handle permission error log_error(f"Permission error for file: {file_path}") return None except Exception as e: # Handle other exceptions log_error(f"An error occurred while reading {file_path}: {str(e)}") return None
def log_error(message): # Log the error with relevant details print(f"ERROR: {message}") # You can also log to a file or external logging service Hope this will be helpful For Everyone, have a nice day :)
The unhandled case represents the case in which a storage read fails because of an external reason (such as failure to read from a database). In this case returning an error is not the correct behaviour, as this will only propagate the error to the cairo program, making it seem like the error was due to the contract's logic rather than an external failure. As we are adding sandboxing for cairo-native execution. Would it make sense to let it panick in this case? And let the sandbox handle it/fallback to cairo-vm. Otherwise this would require refactoring the trait itself to be able to "tell" the native context that the execution has failed due to an external failure. What do you think about this @juanbono?
I'm more in favour of returning a rust Error that can be handled as usual. Because panicking kinds of leak the information that Cairo Native can be run in a sandbox process (and this is not always the case).
