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.