Inconsistent handling of unchecked exception

[manstegling]

This is not a big issue, but the exception handling is a bit confusing.

In a lot of the low-level logic there are methods declaring unchecked exceptions. For example in ValueIn:

@Nullable
Object marshallable(Object object, SerializationStrategy strategy)
        throws **BufferUnderflowException**, **IORuntimeException**;

However, these unchecked exceptions are not propagated consistently in method signatures of higher-level logic.

From this mix of declared and undeclared unchecked exceptions it's very hard to figure out what to expect. Not propagating the exceptions makes it look like they have been handled. However, this is not usually the case.

I guess the easiest improvement would be to add catch clauses to the Chronicle Queue code examples (in the Chronicle-Queue project).

But for the Chronicle-Wire project, it would be nice to add some documentation around the API. For example, is there a specific set of API calls where we can expect unchecked exceptions (e.g. due to corrupt data)? Also, some general documentation around exception throwing strategy (i.e. letting the user take care of any exceptions) would be much appreciated.

Thanks for making great software!

Scenario where it's difficult to understand what to expect

Let's say we do the following (using Chronicle-Queue):

        try (DocumentContext dc = tailer.readingDocument()) {
            if (dc.isPresent()) {
                ValueIn valueIn = dc.wire().getValueIn();
                next = valueIn.object();
        }

In this case we might get a RuntimeException thrown (e.g. if data is corrupt) by the call to valueIn.object() -- which does not declare any exceptions in it's signature.

For example:

Caught throwable: net.openhft.chronicle.core.io.IORuntimeException: Unknown_0x0
at net.openhft.chronicle.wire.BinaryWire$BinaryValueIn.bool(BinaryWire.java:2975)
at net.openhft.chronicle.wire.WireMarshaller$BooleanFieldAccess.setValue(WireMarshaller.java:752)
at net.openhft.chronicle.wire.WireMarshaller$FieldAccess.read(WireMarshaller.java:313)
at net.openhft.chronicle.wire.WireMarshaller.readMarshallable(WireMarshaller.java:143)
at net.openhft.chronicle.wire.Wires.readMarshallable(Wires.java:253)
at net.openhft.chronicle.wire.SerializationStrategies$5.readUsing(SerializationStrategies.java:121)
at net.openhft.chronicle.wire.BinaryWire$BinaryValueIn.marshallable(BinaryWire.java:2897)
at net.openhft.chronicle.wire.ValueIn.object(ValueIn.java:482)
at net.openhft.chronicle.wire.BinaryWire$BinaryValueIn.objectWithInferredType(BinaryWire.java:3158)
at net.openhft.chronicle.wire.ValueIn.object(ValueIn.java:435)

Using version 4.5.27, but as far as I can see it's the same thing in master.

[peter-lawrey]

I agree we need to review our unchecked exceptions. We will add it to the Roadmap. ᐧ

…

On 6 November 2017 at 09:23, manstillman ***@***.***> wrote: This is not a big issue, but the exception handling is a bit confusing. In a lot of the low-level logic there are methods declaring unchecked exceptions. For example in *ValueIn*: @nullable Object marshallable(Object object, SerializationStrategy strategy) throws **BufferUnderflowException**, **IORuntimeException**; However, these unchecked exceptions are not propagated consistently in method signatures of higher-level logic. From this mix of declared and undeclared unchecked exceptions it's very hard to figure out what to expect. Not propagating the exceptions makes it look like they have been handled. However, this is not usually the case. I guess the easiest improvement would be to *add catch clauses* to the Chronicle Queue code examples (in the Chronicle-Queue project). But for the Chronicle-Wire project, it would be nice to add some documentation around the API. For example, is there a specific set of API calls where we can expect unchecked exceptions (e.g. due to corrupt data)? Also, some general documentation around exception throwing strategy (i.e. letting the user take care of any exceptions) would be much appreciated. Thanks for making great software! *Scenario where it's difficult to understand what to expect* Let's say we do the following (using Chronicle-Queue): try (DocumentContext dc = tailer.readingDocument()) { if (dc.isPresent()) { ValueIn valueIn = dc.wire().getValueIn(); next = valueIn.object(); } In this case we might get a RuntimeException thrown (e.g. if data is corrupt) by the call to valueIn.object() -- which does not declare any exceptions in it's signature. For example: Caught throwable: net.openhft.chronicle.core.io.IORuntimeException: Unknown_0x0 at net.openhft.chronicle.wire.BinaryWire$BinaryValueIn.bool( BinaryWire.java:2975) at net.openhft.chronicle.wire.WireMarshaller$BooleanFieldAccess.setValue( WireMarshaller.java:752) at net.openhft.chronicle.wire.WireMarshaller$FieldAccess. read(WireMarshaller.java:313) at net.openhft.chronicle.wire.WireMarshaller.readMarshallable( WireMarshaller.java:143) at net.openhft.chronicle.wire.Wires.readMarshallable(Wires.java:253) at net.openhft.chronicle.wire.SerializationStrategies$5.readUsing( SerializationStrategies.java:121) at net.openhft.chronicle.wire.BinaryWire$BinaryValueIn. marshallable(BinaryWire.java:2897) at net.openhft.chronicle.wire.ValueIn.object(ValueIn.java:482) at net.openhft.chronicle.wire.BinaryWire$BinaryValueIn. objectWithInferredType(BinaryWire.java:3158) at net.openhft.chronicle.wire.ValueIn.object(ValueIn.java:435) Using version 4.5.27, but as far as I can see it's the same thing in master. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#59>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABBU8dvrvBd4Rg6S7qI5HdUoPikjNDqgks5sztAGgaJpZM4QS8NY> .

[manstegling]

Thanks! Obviously, this is nothing pressing. I just figured it might be good to get it on the radar.

As a quick win, perhaps adding a remark to the "Using Chronicle Queue" section of the Chronicle-Queue project README recommending catching RuntimeExceptions in those try-with-resource code examples could address the confusion.

I appreciate the effort that's being put into this project. It's great software!

[dpisklov]

I'm not sure I agree.
The whole point of runtime exceptions (or rather, unchecked exceptions) is that they don't have to be declared in throws block of a method which might throw them. Normally, they happen when there's not much user can do about them, and that's mostly the case in Chronicle code. In the example above, if data is corrupt, one can't do much about it, and hence runtime exception is thrown, describing what's going on. It is up to user, whether he wants to fail fast (do not catch runtime exceptions) or handle them somehow (may be skip the message, recreate the file which stores the wire, etc).
Also this is consistent with Java 8+ approach, where most exceptions became unchecked to be more usable in context of lambdas...

[dpisklov]

I updated doco to mention unchecked exceptions. I don't feel there's anything else to be done here, so closing the issue. Please reopen if disagree.

[hft-team-city]

Released in Chronicle-Wire-2.20.101, BOM-2.20.134