loading from a stream and exceptions #582

user1095108 · 2017-05-13T01:42:12Z

There are 2 possible scenarios when loading from a stream:

there might be an error while loading from the stream (bad sector),
the stream might contain nothing (zero size file),
the json the stream contains might be invalid.

I suppose the library will throw meaningful exceptions if those are enabled, but if disabled, it will abort. There should exist an alternative way to report an error (such as returning an empty json object, for example), without crashing an application.

nlohmann · 2017-05-13T10:04:48Z

The status quo is like this:

#include "json.hpp"
#include <fstream>

using json = nlohmann::json;

int main()
{
    std::fstream f_non("nonexisting.json");
    std::fstream f_empty("empty.json");
    std::fstream f_error("error.json");
    
    try {
        json::parse(f_non);
    } catch (json::parse_error &e)
    {
        std::cerr << e.what() << std::endl;
    }

    try {
        json::parse(f_empty);
    } catch (json::parse_error &e)
    {
        std::cerr << e.what() << std::endl;
    }

    try {
        json::parse(f_error);
    } catch (json::parse_error &e)
    {
        std::cerr << e.what() << std::endl;
    }
}

Output:

[json.exception.parse_error.111] parse error: bad input stream
[json.exception.parse_error.101] parse error at 1: syntax error - unexpected end of input
[json.exception.parse_error.101] parse error at 1: syntax error - invalid literal; last read '@'

That is, in all three cases (nonexisting file, empty file, file with syntax error), a json::parse_error exception is thrown. In case exceptions are switched off, all three cases call std::abort().

user1095108 · 2017-05-13T15:24:10Z

Yes, but the application programmer has no way to notify the user of an error, if you abort. This is something I'd like to be able to do in my app.

user1095108 · 2017-05-13T15:25:06Z

You could return an empty object or even use std::optional.

user1095108 · 2017-05-13T15:26:19Z

Or std::pair containing an object and bool. The bool indicating an error.

nlohmann · 2017-05-13T15:35:41Z

A validity check will be implemented with #458. This would avoid the last 2 exceptions. The first exception is thrown in the input_adapter class - I have to check whether it makes sense to remove it there. This could avoid refactoring the parser methods.

Note that std::optional is not part of C++11, and stepping forward to C++17 would exclude too many users.

user1095108 · 2017-05-13T15:48:27Z

You still have std::pair then

user1095108 · 2017-05-13T15:49:09Z

Also, hacking your own std::optional is not too difficult

nlohmann · 2017-05-13T15:50:52Z

I'm not convinced yet whether changing the signature of the parse function is worth it in the first place.

user1095108 · 2017-05-13T16:06:31Z

Of course, you should introduce another function. But really, what bothers me most is, if I "pipe" a stream into a Json object your library might abort or throw. This is completely wrong. You should check if exceptions are enabled in the stream before throwing or aborting. You can set flags in the stream, you don't need to throw or abort.

nlohmann · 2017-05-13T16:09:57Z

I understand.

JakeShirley · 2017-05-15T16:24:35Z

I too would appreciate being able to parse without using exceptions. 👍

nlohmann · 2017-06-17T12:12:29Z

Opened #623 as place to discuss the structure of the parsing functions.

You can now pass a boolean "allow_exceptions" to the parse functions. If it is false, no exceptions are thrown in case of a parse error. Instead, parsing is stopped at the first error and a JSON value of type "discarded" (check with is_discarded()) is returned.

nlohmann · 2017-07-27T19:03:40Z

You can now pass a boolean allow_exceptions to the parse functions. If it is false, no exceptions are thrown in case of a parse error. Instead, parsing is stopped at the first error and a JSON value of type "discarded" (check with is_discarded()) is returned.

user1095108 · 2017-07-27T19:06:23Z

This is very nice, but does it work with streams? I mean, when you pipe a json object out of a stream. I propose an iostream manipulator. json j; stream >> j; 2017-07-27 21:03 GMT+02:00 Niels Lohmann <[email protected]>:

…

You can now pass a boolean allow_exceptions to the parse functions. If it is false, no exceptions are thrown in case of a parse error. Instead, parsing is stopped at the first error and a JSON value of type "discarded" (check with is_discarded()) is returned. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#582 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AH6jVLCxM-nGNWiBRR_Upacb0G2MYYIVks5sSN8YgaJpZM4NZ7OA> .

nlohmann · 2017-07-27T19:09:20Z

Piping does not work, because you cannot provide an additional parameter. You need to use the parse function:

json j = json::parse(stream, nullptr, false);

(The nullptr means "no callback function" and false means "no exceptions").

((But I could switch off exceptions automatically when JSON_NOEXCEPTION is defined.))

user1095108 · 2017-07-27T19:19:15Z

Fair enough, I had something like this in mind stream >> json::no_exceptions >> j; an io manipulator, look here for reference: http://www.cplusplus.com/reference/iomanip/ 2017-07-27 21:09 GMT+02:00 Niels Lohmann <[email protected]>:

…

Piping does not work, because you cannot provide an additional parameter. You need to use the parse function: json j = json::parse(stream, nullptr, false); (The nullptr means "no callback function" and false means "no exceptions"). ((But I could switch off exceptions automatically when JSON_NOEXCEPTION is defined.)) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#582 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AH6jVMnxzBQuZQTFFbrOm3HBvUvArd1Hks5sSOBqgaJpZM4NZ7OA> .

nlohmann · 2017-07-27T19:42:18Z

Such a manipulator would be syntactic sugar and I am not sure yet whether it would justify the effort.

user1095108 · 2017-07-27T19:47:56Z

The whole point of C++ is syntactic sugar, that's why I made my suggestion. 2017-07-27 21:42 GMT+02:00 Niels Lohmann <[email protected]>:

…

Such a manipulator would be syntactic sugar and I am not sure yet whether it would justify the effort. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#582 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AH6jVDVHlM8VaJ57rtk7VEPjiNmBLlppks5sSOgggaJpZM4NZ7OA> .

nlohmann · 2017-07-27T19:49:11Z

I know - but in the end, this is a lot of work for a feature that (probably) hard anyone uses.

user1095108 · 2017-07-27T19:51:43Z

You have many different options, some more difficult than others to implement: stream >> json::no_exceptions(j); might be easier. 2017-07-27 21:49 GMT+02:00 Niels Lohmann <[email protected]>:

…

I know - but in the end, this is a lot of work for a feature that (probably) hard anyone uses. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#582 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AH6jVDdph_4id2Gouo6TPaRgUGJu79Q-ks5sSOnEgaJpZM4NZ7OA> .

nlohmann · 2017-07-27T19:53:59Z

Yes. But the parse function is much more versatile. For instance, the >> operator does not support a callback function. Even if I would implement all this, write proper tests and documentation, I think that it would mean a lot of development and maintenance work.

JakeShirley · 2017-07-28T15:18:59Z

@nlohmann excellent work! I greatly appreciate your dedication to this library :)

user1095108 · 2017-07-30T12:59:40Z

It's been so long, that I forgot to remind you, that there is a flag within the stream, that tells you if you don't want it to throw exceptions:

http://en.cppreference.com/w/cpp/io/basic_ios/exceptions

you can simply check that and act accordingly, no need for io manipulators.

nlohmann · 2017-07-30T16:18:01Z

All streams have goodbit by default (they do not throw exceptions due to error state flags being set).

If I would use these flags, then I would change the behavior of the stream operators, because they would not throw unless explicitly specified.

user1095108 · 2017-07-30T18:35:26Z

But that's what they are for, enabling or disabling exception throwing. I didn't mean you should set any flags in the stream, just query the exception flag. 2017-07-30 18:18 GMT+02:00 Niels Lohmann <[email protected]>:

…

All streams have goodbit by default (they do not throw exceptions due to error state flags being set). If I would use these flags, then I would change the behavior of the stream operators, because they would not throw unless explicitly specified. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#582 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AH6jVIx4ta6ySIqxidLaLsPt19p5I67Xks5sTKy-gaJpZM4NZ7OA> .

nlohmann · 2017-07-30T19:11:57Z

Yes. But if I would query the exception flag (which are not set by default) and only throw exceptions on parse errors if the flags are set, then existing code would behave differently. I think this would be an issue.

user1095108 · 2017-07-30T19:25:36Z

Your stated goal is to follow STL conventions, existing behavior is therefore a bug, inconsistent with your goal. 2017-07-30 21:12 GMT+02:00 Niels Lohmann <[email protected]>:

…

Yes. But if I would query the exception flag (which are not set by default) and only throw exceptions on parse errors if the flags are set, then existing code would behave differently. I think this would be an issue. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#582 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AH6jVOg_mfnQ08KHuiQJ8czjYFtK0cS1ks5sTNWCgaJpZM4NZ7OA> .

user1095108 · 2017-07-30T19:26:45Z

also, in truth, you would only change the behavior of the >> operator, not a big deal, in my opinion. Most code probably uses parse(). 2017-07-30 21:25 GMT+02:00 janezz55 . <[email protected]>:

…

Your stated goal is to follow STL conventions, existing behavior is therefore a bug, inconsistent with your goal. 2017-07-30 21:12 GMT+02:00 Niels Lohmann ***@***.***>: > Yes. But if I would query the exception flag (which are not set by > default) and only throw exceptions on parse errors if the flags are set, > then existing code would behave differently. I think this would be an issue. > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#582 (comment)>, or mute > the thread > <https://github.com/notifications/unsubscribe-auth/AH6jVOg_mfnQ08KHuiQJ8czjYFtK0cS1ks5sTNWCgaJpZM4NZ7OA> > . >

nlohmann · 2017-07-30T19:32:32Z

I unfortunately do not have any statistics about which functions are used how frequently. And I fear that "fixing" this behavior yields a lot of support questions.

user1095108 · 2017-07-30T19:37:40Z

I don't think so, in fact I think it will be the opposite. Lots of people wished to avoid exception throwing within the library and no one can foretell the future anyway. 2017-07-30 21:32 GMT+02:00 Niels Lohmann <[email protected]>:

…

I unfortunately do not have any statistics about which functions are used how frequently. And I fear that "fixing" this behavior yields a lot of support questions. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#582 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AH6jVFe5aFQ4QbmAiRAbbs6ciXC4sYRkks5sTNpVgaJpZM4NZ7OA> .

user1095108 · 2017-07-30T19:44:55Z

and really, it is an inconsistency. Say you were reading a float out of the stream and it was in the wrong format. The bad bit would be set and possibly an exception throw and if the json is wrong, the same should happen. 2017-07-30 21:37 GMT+02:00 janezz55 . <[email protected]>:

…

I don't think so, in fact I think it will be the opposite. Lots of people wished to avoid exception throwing within the library and no one can foretell the future anyway. 2017-07-30 21:32 GMT+02:00 Niels Lohmann ***@***.***>: > I unfortunately do not have any statistics about which functions are used > how frequently. And I fear that "fixing" this behavior yields a lot of > support questions. > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#582 (comment)>, or mute > the thread > <https://github.com/notifications/unsubscribe-auth/AH6jVFe5aFQ4QbmAiRAbbs6ciXC4sYRkks5sTNpVgaJpZM4NZ7OA> > . >

nlohmann · 2017-07-30T19:46:27Z

I understand your point. But please also understand that changing the behavior of existing code is also an issue. I really would like to have a broader discussion here.

user1095108 · 2017-07-30T20:07:23Z

I doubt there will be many comments about this issue. 2017-07-30 21:46 GMT+02:00 Niels Lohmann <[email protected]>:

…

I understand your point. But please also understand that changing the behavior of existing code is also an issue. I really would like to have a broader discussion here. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#582 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AH6jVLou_zQW76AG0tOIBxephQ_Wfup7ks5sTN2YgaJpZM4NZ7OA> .

nlohmann · 2017-08-15T18:53:46Z

I close this issue without change. Breaking existing code is not an option.

user1095108 · 2017-08-15T19:02:42Z

I can only say to this: to hell with your library.

nlohmann added the kind: question label May 13, 2017

nlohmann added the state: please discuss please discuss the issue or vote for your favorite option label May 13, 2017

nlohmann added solution: proposed fix a fix for the issue has been proposed and waits for confirmation and removed state: please discuss please discuss the issue or vote for your favorite option labels Jul 27, 2017

nlohmann added the state: please discuss please discuss the issue or vote for your favorite option label Jul 30, 2017

nlohmann closed this as completed Aug 15, 2017

gregmarr mentioned this issue Aug 30, 2017

How to detect parse failure? #715

Closed

tomasMizera mentioned this issue Oct 6, 2017

Program calls abort function #769

Closed

loading from a stream and exceptions #582

loading from a stream and exceptions #582

Comments

user1095108 commented May 13, 2017 • edited Loading

nlohmann commented May 13, 2017

user1095108 commented May 13, 2017

user1095108 commented May 13, 2017

user1095108 commented May 13, 2017

nlohmann commented May 13, 2017

user1095108 commented May 13, 2017

user1095108 commented May 13, 2017

nlohmann commented May 13, 2017

user1095108 commented May 13, 2017

nlohmann commented May 13, 2017

JakeShirley commented May 15, 2017

nlohmann commented Jun 17, 2017

nlohmann commented Jul 27, 2017

user1095108 commented Jul 27, 2017 via email

nlohmann commented Jul 27, 2017

user1095108 commented Jul 27, 2017 via email

nlohmann commented Jul 27, 2017

user1095108 commented Jul 27, 2017 via email

nlohmann commented Jul 27, 2017

user1095108 commented Jul 27, 2017 via email

nlohmann commented Jul 27, 2017

JakeShirley commented Jul 28, 2017

user1095108 commented Jul 30, 2017

nlohmann commented Jul 30, 2017

user1095108 commented Jul 30, 2017 via email

nlohmann commented Jul 30, 2017

user1095108 commented Jul 30, 2017 via email

user1095108 commented Jul 30, 2017 via email

nlohmann commented Jul 30, 2017

user1095108 commented Jul 30, 2017 via email

user1095108 commented Jul 30, 2017 via email

nlohmann commented Jul 30, 2017

user1095108 commented Jul 30, 2017 via email

nlohmann commented Aug 15, 2017

user1095108 commented Aug 15, 2017

user1095108 commented May 13, 2017 •

edited

Loading