Add Key name to Exception

[tcanabrava]

Bug Report

(I'm interested in developing this feature, so pointers are appreciated)

• When working with an invalid json, the exception produced is not really helpfull.

I'v found out #160 - but that's a different issue than the one I'm having, and it will also be harder to fix because the values have no notion of key, the specific code I'm talking about on json.hpp follows this pattern:

template<typename BasicJsonType>
void from_json(const BasicJsonType& j, typename BasicJsonType::string_t& s)
{
    if (JSON_UNLIKELY(not j.is_string()))
    {
        JSON_THROW(type_error::create(302, "type must be string, but is " + std::string(j.type_name())));
    }
    s = *j.template get_ptr<const typename BasicJsonType::string_t*>();
}

but even if we have a small json to handle, the output is not really helpfull:

std::exception: [json.exception.type_error.302] type must be string, but is number

• Small code example:

nlohmann::json j {{"firstElement", ""}};
j["firstElement"] = 0;

• What is the expected behavior?
  std::exception: [json.exception.type_error.302] type for key firstElement must be string, but is number

• Did you use a released version of the library or the version from the develop branch?

3.0.1

[nlohmann]

Without looking too deep into this issue, I fear that we do not have this information at this point.

[tcanabrava]

yes, we do not, I stated that on the bug report. the question is more like "how hard it's to refactor the code to have the information". I tried to hack around the source but I was a bit lost with all the template magic around...

[nlohmann]

Channing the signature of from_json touches too many other aspects of the code, so I fear this is not feasible.

Then I could think about catching the 302 exception by the caller (operator[] in your example) and re-throwing it with a new description. I am not sure about the costs of this, compared to the benefit.

[tcanabrava]

That's one of the points I hate exception :)

I catched (...) on the operator[] for nlohmann::object and rethrowed using the key, but I got the original throw, that means that my guess on where the exception is being trown is wrong or it's happening after the return from the operator[] - and for that I'd also lose the key.

Any other pointers on how should I proceed?

[nlohmann]

Conceptually, we always face the problem that a JSON value does not know where it is stored, so you can never query the key of a value in an object - you don't even know whether the current value is actually stored inside another value.

Another question: What would you report for nested keys, for instance:

json j;
j["foo"]["bar"]["baz"] = 1;
j["foo"]["bar"]["baz"]["key"] = true;

throwing [json.exception.type_error.305] cannot use operator[] with number.

Even if you would report key - what about foo, bar, and baz?

[gregmarr]

The exception would need to be caught at each stage walking back up the chain, the new key added to the front of the path, and then thrown again. Unfortunately in this case, it's too late for that, as the rest of the chain is gone. This would only work for parsing, but that's better than nothing.

[nlohmann]

@gregmarr Right - this is my point. Question is: what would be the price for this and would one be willing to pay it?

[tcanabrava]

on your example I think it should report "baz" and not "key", as the 'number' in question is stored in "baz".
and I think that would help the developer to catch the error on their json quite a lot, and I would be really happy with that solution.

Other approach is to store the key as a string_view (considering that all keys are strings) on every value so the value can know who holds itself, and then on throw we display that.

about the fully qualified namespace on throw 'foo.bar.baz' , I think that's nice but I wouldn't mind just 'baz' to start.

[nlohmann]

Adding a key would mean overhead for every JSON value - even if it is not stored inside an object.

[tcanabrava]

But the key already exists, as it's used in the holder object, if we use string_view the overhead will be that big?
(I'm thinkering here with the repository testing stuff, I hope to have some code to show soon)

[nlohmann]

But the string view also takes some overhead (note: C++11 has no string view), and this overhead is per value.

[tcanabrava]

a try that I know it's wrong for the library as I used a external variable, but if you feel that this could be somewhat userfull I'll rework to merge on the json lib.

external std::string last_used_key;

on the operator[] for the objects:

    const_reference operator[](const typename object_t::key_type& key) const
    {
				last_used_key = key;
                               ....
    }

on the exception:

protected:
    exception(int id_, const char* what_arg)
		: id(id_), what_error(std::string(what_arg) + " Possible Key: " + last_used_key), m(what_error.c_str())
		 {
		 }

This adds for me what I wanted, overhead is negligible wiht my tests and the result of the exception is better:

[json.exception.type_error.302] type must be string, but is number. Possible Key: cardType

[nlohmann]

Note you always make a copy of the key for this. I agree that this would solve the issue, but I do not like the idea of maintaining state inside the library as this may bring too many issues down the road.

[tcanabrava]

I agree that it's a poor man's solution to my issue. Also I'm currently out of ideas on how to tackle this. for me this is a real issue as I'm dealing with the conversion from c based structs to json and sometimes I use the wrong type on the json field leading to exceptions without telling me the key in question.
I think another possibility that doesn't involves how the json library work is to attach a backtrace on the output on the what() of the exception using boost stacktrace if available.

[gregmarr]

Maybe a compromise between keeping the size down and the ability to report the stack is that if NDEBUG is not defined, then basic_json<> contains a basic_json<> *parent; that is set when an array or object adds the object to its collection. That's the minimum size that would allow you to walk back up the chain. The function that throws the exception can then walk the parent chain to rebuild the JSON value path.

[matthewOConnell]

In my projects json access isn't a performance concern. And certainly nothing compared to the cost of debugging when an input string isn't as expected. A define to allow choice would be a godsend.

I would suggest to not use a common compiler flag though. I don't want to have to use _DEBUG or NDEBUG everywhere to get the error messages. Maybe something like JSON_VERBOSE_EXCEPTIONS that gets toggled by a standard debug flag but could also be set independently.

[gregmarr]

One potential problem with a define like that is that you need to make sure that you define it the same way everywhere, or you have ODR violations. However, there are already other defines that would have the same effect, so maybe that's not an issue.

[nlohmann]

I have two questions regarding #932 (comment):

• It seems that every function like push_back or insert needs to be touched to assign this to the value to be added to the container. Any idea how to cope with functions like emplace(Args&& ... args) where we call m_value.object->emplace(std::forward<Args>(args)...) internally?
• Functions like operator[] do not add values itself, but only return references to which we assign. How to add a parent pointer there?

[gregmarr]

Looks like the emplace/push_back/insert functions all return an iterator. It would probably require using that iterator to get access to the new object, and if it's a basic_json<> object, set the parent pointer.

Functions like operator[] which just eventually defer directly to the map or vector class:

return m_value.array->operator[](idx);

would need modification to do something like this:

reference result = m_value.array->operator[](idx);
// set the pointer
return result;

I agree that it would take a fair amount of work, but it should be doable if someone wanted to propose it.

[nlohmann]

Thanks! Seeing all this, this would mean a lot of adjustments and #ifdef blocks in a lot of functions...

[gregmarr]

It could theoretically be done with a small set of helper functions that contain the #ifdef blocks, but yes, a lot of adjustments.

[nlohmann]

I won't be implementing this in the foreseeable future, because I cannot think of an approach that means a lot of adjustments. PRs welcome!

[logidelic]

I know you said that you won't be implementing this, but I just came by to say "me too".

I've been using nlohmann::json for a few years and, it is indeed extraordinarily awesome. However, this one "little" issue drives me bonkers on a regular basis. I understand that we're talking about walking the nested keys, etc, but even just having the leaf key name would be massively useful.

My idea: store the last accessed key in TLS and include it in this exception. Hackish and ugly? For sure, but better than living in the dark... :)

[nlohmann]

Maybe the future brings a nice idea for this. Until then, I close the issue.

[TrueWodzu]

Hi,

It is my first day with this library and I must admit I've spent few hours trying to output key name before I've found this issue. I must say it is a shame that this won't be implemented, because being able to tell what key is "broken" is a crucial feature not only for a developer but also for a user who will be reading log messages.

[nlohmann]

@TrueWodzu As you may see in the discussion, there is no easy solution to this issue. :-/

[TrueWodzu]

@nlohmann Yes:( I do have a small proposal though which might help to pinpoint offending value. I know that value does not know about the key, but maybe it would be a good idea to include value's underlying data (if it is of a simple type) itself? For example:

struct person {
	std::string name;
	std::string address;
	int age;
};

json j = R"({"address":"744 Evergreen Terrace","age":"60","name":"Ned Flanders"})"_json;

try
{
	person p;
	p.age = j.at("age").get<int>(); //p.age should be int but json contains string
}
catch (const nlohmann::detail::exception& exc)
{
	std::cout << exc.what() << "\n";
}

Now the output is:
[json.exception.type_error.302] type must be number, but is string

and it could be:
[json.exception.type_error.302] type must be number, but is string ["60"]

Do you think, that it would be possible without any additional overhead?
By knowing a value it would be easier for someone to find problem in json file.

[logidelic]

FWIW, I ended up writing a template helper function to wrap the exception. I still think it should be included by default, but at least this works:

// get - json helper; gets value of key or throws
template<class T>
T get(const json& j, string key) {
    auto it = j.find(key);
    if(it == j.end())
        throw runtime_error("get(json) - key not found - "+key);

    try {
        return it->get<T>();
    } catch(const exception& e) {
        throw runtime_error("get(json) - get-exception - "+key+" - "+e.what());
    }
}

[tcanabrava]

that will not work if you are trying to access via multiple keys: some_string = blah["first_key"]["second_key"]; so this fix actually is for the simplest case.

…

On Thu, Jun 28, 2018 at 3:17 PM, Logidelic ***@***.***> wrote: FWIW, I ended up writing a template helper function to wrap the exception. I still think it should be included by default, but at least this works: // get - json helper; gets value of key or throws template<class T> T get(const json& j, string key) { auto it = j.find(key); if(it == j.end()) throw runtime_error("get(json) - key not found - "+key); try { return it->get<T>(); } catch(const exception& e) { throw runtime_error("get(json) - get-exception - "+key+" - "+e.what()); } } — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#932 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AD1zUKVz4srRh74_9YEhUZuX9rP6ddE9ks5uBNdsgaJpZM4RpZYx> .

[nlohmann]

As the discussion lives on here after such a long, time, I had a look at the example from #932 (comment) in my IDE.

I got the following hints from the debugger:

The first view shows me the line in my code that lead to the issue. So I know exactly where the access with the type error occurred. The second view shows me the function where I see the object key.

I can understand your request for more detailed exception messages, but the exception itself gives you the guidance you need.

Or what am I missing?

[tcanabrava]

ignore the first line as there you know that you added a number and it's easy to see. imagine that you fetched the json from a webserver and that they changed the schema for some reason and what used to work now doesn't.
then you do:

nlohmann::json j = nlohmann::json::parse(webserver_json_string);
... some lines of json code...
j["foo"]["bar"]["baz"]["key"] = true;
...some lines of json code ...

can you easily say if the problem is with objec "foo", "bar", "baz" or "key" ?

Another problem in my view is that the many real world cases cannot be tested in an IDE (plugins of applications come up to mind, or ZeroC - Ice servers, that's my case.

Also in applications that cannot die - where the throw is catched and the program never dies, like many corporated systems you cannot rely in the backtrace that the throw would do if the program crashes, so if you caugth the exception and throw it to a log (like what a webserver would do) you are hopeless: there's probabely too many json usages for you to discover wich one failed.

[TrueWodzu]

@logidelic Thanks for the example, I think the first case (when key is missing) have been already implemented.

@nlohmann My point of view is from end user perspective, not a programmer. This end user is a guy from support, responsible for installing my service on client's machine and supporting it.

Imagine that my application needs a configuration file and I will store my configuration in json. People from support will edit this configuration from time to time and make mistakes. My application will catch an exception from json library and write it to the log. At its current state, exception does not give a lot of information i.e. what key/value pair of json file is wrong. So people from support won't be able to fix it by themselves and they will create an issue in bugtracker for me (rightfully so) and I would have to look into it.

Because exception does not contain information what value for what key is wrong I can't create a nice human readable message for support guys and that makes work of everyone harder.

I understand that now it is very hard to incorporate such change, I just wanted to give you a different point of view. It is hard to create a user friendly application if I can't show user where the problem is.

As @logidelic has shown there is a workaround but it is a bit cumbersome to use.

[TrueWodzu]

nlohmann::json j = nlohmann::json::parse(webserver_json_string);
... some lines of json code...
j["foo"]["bar"]["baz"]["key"] = true;
...some lines of json code ...

can you easily say if the problem is with objec "foo", "bar", "baz" or "key" ?

Correct me if I am wrong, but I think exception happens as you go down of a path, so firstly you parse foo then bar then "baz" and so on. So for example if bar is no longer an object but it is a number then exception will happen at bar level.

In such case I would expected an exception saying something along these lines "Expected number but got object for key "bar"".

I don't have any experience with the library so I could be wrong im my reasoning.

[tcanabrava]

TrueWodzu <notifications@github.com> schrieb am Do. 28. Juni 2018 um 16:42:

nlohmann::json j = nlohmann::json::parse(webserver_json_string); ... some lines of json code... j["foo"]["bar"]["baz"]["key"] = true; ...some lines of json code ... can you easily say if the problem is with objec "foo", "bar", "baz" or "key" ? Correct me if I am wrong, but I think exception happens as you go down of a path, so firstly you parse foo then bar then "baz" and so on. So for example if bar is no longer an object but it is a number then exception will happen at bar level. In such case I would expected an exception saying something along these lines "Expected number but got object for key "bar"".

Right now the library says “expected number but got object”, without any key. I don't have any experience with the library so I could be wrong im my

…

reasoning. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#932 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AD1zUHNmzIzPk5MSA9qR-yrJSXHAWxasks5uBOtXgaJpZM4RpZYx> .

[nlohmann]

I see. I shall have another look.

[nlohmann]

FYI: In #1152 is a broader discussion on exceptions and their messages.

[TrueWodzu]

Thanks, I will take a look.

[zhongjingjogy]

Thanks for the great Json library.

May I ask whether there is a solution for adding debug information about the key? I have use nlohmann::json heavily in my project, and I frequently got trapped in finding the missing keys in a complicated json file. I came across issues about this topic but I am not able to figure out a less painful solution.

I am looking forward to some suggestions.

Thanks

[bvnp44]

[json.exception.type_error.302] type must be number, but is string this thing makes me crazy when you work with external server that could send any response

[nlohmann]

FYI: I am trying to fix this issue in #2562. Any help would be greatly appreciated!