Adopt a system of exceptions derived from KaitaiStructError #80
Conversation
* Resolves #40 * Resolves #41 As explained in #40, this makes it easy to handle to all errors caused by invalid input data by using `kaitaistruct.KaitaiStructError` in a `try..except` statement. Three new exception types were added: `InvalidArgumentError`, `EndOfStreamError` and `NoTerminatorFoundError`. All changes to raised exceptions in this commit should be backward compatible, as we are only moving to subclasses of previously raised exceptions. `NoTerminatorFoundError` is a subclass of `EndOfStreamError` to address the suggestion in #41. Note that the `process_rotate_left` method could only raise `NotImplementedError` if someone called it manually (because KSC-generated parsers hardcode `group_size` to `1`, see https://github.com/kaitai-io/kaitai_struct_compiler/blob/c23ec2ca88d84042edba76f70c1f003d062b7585/shared/src/main/scala/io/kaitai/struct/languages/PythonCompiler.scala#L211), so it makes no sense to raise an exception derived `KaitaiStructError` (it's a programmer error, not a user input error). Most of our runtime libraries in other languages don't even have this `group_size` parameter, and if they do (C#, Java, Ruby), they also throw the equivalent of `NotImplementedError` (except the JavaScript runtime, which throws a plain string, which is _possible_ in JS but considered bad practice, so we should fix this).
| @@ -128,6 +128,9 @@ def is_eof(self): | |||
| return self._io.tell() >= self.size() | |||
|
|
|||
| def seek(self, n): | |||
| if n < 0: | |||
| raise InvalidArgumentError("cannot seek to invalid position %d" % (n,)) | |||
There was a problem hiding this comment.
For the record, this exception was already thrown by the io.IOBase.seek() method in the Python standard library. The exception type and message text and depended on which underlying stream was used. When using io.BytesIO, a ValueError("negative seek value %zd") was thrown (see Modules/_io/bytesio.c:659-660). But when using a regular file, I was getting OSError: [Errno 22] Invalid argument on both Windows and Linux.
So it seems better to do this check ourselves, if only for the sake of consistent behavior. And also because a negative seek position can easily be a result of invalid input data (and thus the raised exception should be a subclass of KaitaiStructError) - imagine a situation like this:
meta:
id: negative_seek
seq:
# Consider parsing a byte with the highest bit set, i.e. `80..ff` in hex
- id: ofs_foo
type: s1
instances:
foo:
pos: ofs_foo
type: u1If ofs_foo is negative and we attempt to read foo, the error that occurs is a clear parse error.
|
I'm going to merge this now, but feel free to comment on this PR even after the merge. |
As explained in #40, this makes it easy to handle to all errors caused by invalid input data by using
kaitaistruct.KaitaiStructErrorin atry..exceptstatement. Three new exception types were added:InvalidArgumentError,EndOfStreamErrorandNoTerminatorFoundError. All changes to raised exceptions in this commit should be backward compatible, as we are only moving to subclasses of previously raised exceptions.NoTerminatorFoundErroris a subclass ofEndOfStreamErrorto address the suggestion in #41.Note that the
process_rotate_leftmethod could only raiseNotImplementedErrorif someone called it manually (because KSC-generated parsers hardcodegroup_sizeto1, see https://github.com/kaitai-io/kaitai_struct_compiler/blob/c23ec2ca88d84042edba76f70c1f003d062b7585/shared/src/main/scala/io/kaitai/struct/languages/PythonCompiler.scala#L211), so it makes no sense to raise an exception derivedKaitaiStructError(it's a programmer error, not a user input error). Most of our runtime libraries in other languages don't even have thisgroup_sizeparameter, and if they do (C#, Java, Ruby), they also throw the equivalent ofNotImplementedError(except the JavaScript runtime, which throws a plain string, which is possible in JS but considered bad practice, so we should fix this).