Json: loading from a stream and exceptions

Created on 13 May 2017  ยท  35Comments  ยท  Source: nlohmann/json

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.

question proposed fix please discuss

Most helpful comment

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

All 35 comments

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().

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.

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

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

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.

You still have std::pair then

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

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

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.

I understand.

I too would appreciate being able to parse without using exceptions. ๐Ÿ‘

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.

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 notifications@github.com:

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
https://github.com/nlohmann/json/issues/582#issuecomment-318456711, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AH6jVLCxM-nGNWiBRR_Upacb0G2MYYIVks5sSN8YgaJpZM4NZ7OA
.

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.))

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 notifications@github.com:

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
https://github.com/nlohmann/json/issues/582#issuecomment-318458245, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AH6jVMnxzBQuZQTFFbrOm3HBvUvArd1Hks5sSOBqgaJpZM4NZ7OA
.

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

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 notifications@github.com:

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
https://github.com/nlohmann/json/issues/582#issuecomment-318465936, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AH6jVDVHlM8VaJ57rtk7VEPjiNmBLlppks5sSOgggaJpZM4NZ7OA
.

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

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 notifications@github.com:

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
https://github.com/nlohmann/json/issues/582#issuecomment-318467452, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AH6jVDdph_4id2Gouo6TPaRgUGJu79Q-ks5sSOnEgaJpZM4NZ7OA
.

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.

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

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.

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.

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 notifications@github.com:

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
https://github.com/nlohmann/json/issues/582#issuecomment-318912213, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AH6jVIx4ta6ySIqxidLaLsPt19p5I67Xks5sTKy-gaJpZM4NZ7OA
.

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.

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 notifications@github.com:

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
https://github.com/nlohmann/json/issues/582#issuecomment-318922802, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AH6jVOg_mfnQ08KHuiQJ8czjYFtK0cS1ks5sTNWCgaJpZM4NZ7OA
.

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 . janezz55@gmail.com:

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 notifications@github.com:

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
https://github.com/nlohmann/json/issues/582#issuecomment-318922802, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AH6jVOg_mfnQ08KHuiQJ8czjYFtK0cS1ks5sTNWCgaJpZM4NZ7OA
.

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.

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 notifications@github.com:

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
https://github.com/nlohmann/json/issues/582#issuecomment-318923923, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AH6jVFe5aFQ4QbmAiRAbbs6ciXC4sYRkks5sTNpVgaJpZM4NZ7OA
.

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 . janezz55@gmail.com:

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 notifications@github.com:

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
https://github.com/nlohmann/json/issues/582#issuecomment-318923923, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AH6jVFe5aFQ4QbmAiRAbbs6ciXC4sYRkks5sTNpVgaJpZM4NZ7OA
.

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.

I doubt there will be many comments about this issue.

2017-07-30 21:46 GMT+02:00 Niels Lohmann notifications@github.com:

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
https://github.com/nlohmann/json/issues/582#issuecomment-318924711, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AH6jVLou_zQW76AG0tOIBxephQ_Wfup7ks5sTN2YgaJpZM4NZ7OA
.

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

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

Was this page helpful?
0 / 5 - 0 ratings

Related issues

zkelo picture zkelo  ยท  3Comments

MariaRamos89 picture MariaRamos89  ยท  4Comments

afowles picture afowles  ยท  3Comments

moneroexamples picture moneroexamples  ยท  4Comments

edi9999 picture edi9999  ยท  3Comments