Support CBOR parsing using error codes

[bryceschober]

It appears to me that all CBOR parsing failures are now requiring exceptions, since #325 was merged. It appears to me that all of the current decode_cbor() implementations require exceptions or aborts. The underlying decode() and read() functions do at least have error-code-oriented variants, but in the decode_cbor() implementations, those are all turned into exceptions, AFAICT.

Without digging more deeply than my C++ expertise would easily go, I can't tell which of those are used by reflect-cpp. I also can't discern how much effort it might be for reflect-cpp to use the underlying error-code-oriented methods.

But as is, the Result and monadic features of reflect-cpp aren't really available to CBOR users.

[liuzicheng1987]

I think that out CBOR wrapper uses monadic error handling just like all the other formats.

You can check out include/cbor/Reader.hpp and you will find that we do a lot of checks using the exact same logic we use for the other formats. You will also find that there is frequent use of noexcept, meaning any exception would not be propagated. We also sometimes use a try...except logic which would catch CBOR exceptions.

Or is there a specific situation where the current set up is creating difficulties?

[bryceschober]

I will see if I can trim down my private use-case, in switching from about v0.15.0 to latest, into a public test case PR.

[bryceschober]

See my draft PR #432. I don't know how to work out the correct solution, but I committed a hacky-crap "sledgehammer" approach.

[liuzicheng1987]

@bryceschober , in my view your "hacky-crap sledgehammer" approach is totally fine. I see nothing wrong with it.

[bryceschober]

@bryceschober , in my view your "hacky-crap sledgehammer" approach is totally fine. I see nothing wrong with it.

My "problem" with it is that jsoncons is generating a std::error_code code, then in the interface that we're using, it turns it into an exception, and then we turn it back into an error object, though not of the "standard" variety.

I don't know in practice which variants of decode_cbor() are actually being used by rfl::cbor::read() , nor do I have the expertise to understand the templates, etc. required to understand how I would rewrite it to use the available std::error_code interfaces of jsoncons to avoid the unnecessary throw-catch.

I do see now that it would be constructing an exception object regardless, and while it seems like it would be more efficient to convert the exception object into a rfl::Error, I guess I do agree that jsoncons doesn't look to be designed to support any non-exception-style of error handling akin to std::expected.

All of that said, In order to better support your monadic operations, a modified version of my PR that is cleaned up and also covers other usages of jsoncons is probably the best for now.

[bryceschober]

The upstream danielaparker/jsoncons#615 non-throwing support has landed. I will add to my backlog the prototyping of an updated solution using the new try_decode_*() interfaces on the implementations that use jsoncons.

[bryceschober]

The upstream danielaparker/jsoncons#615 non-throwing support has landed. I will add to my backlog the prototyping of an updated solution using the new try_decode_*() interfaces on the implementations that use jsoncons.

I've finally started work on this, and I think I've gotten a workable concept started, that can detect the new function and provide alternate rfl::cbor::read() implementations depending on the detected support function.

I'm running into a problem being unfamiliar with vcpkg development workflows, though. Because jsoncons hasn't yet released a version with the merged try_decode_*() functions, I don't know how I would get the reflect-cpp build infrastructure to compile against current jsoncons/master. I would develop and get a draft PR going, but I wouldn't intend that it be ready & mergeable until they did tag a version. On the other hand, my downstream build method doesn't depend on a tagged version, and I'd like to be able to test against both current and future versions.

@liuzicheng1987, do you have any suggestions?

[liuzicheng1987]

@bryceschober I would wait until there is an official release on vcpkg and Conan. Otherwise it's just too much hassle.

[bryceschober]

Resolved sufficiently with the merge of #550. Thanks, @Ian-Flury!