glaze icon indicating copy to clipboard operation
glaze copied to clipboard

Enhance `missing_key` error message

Open pfirsich opened this issue 1 year ago • 3 comments

Currently the error message for missing keys is simply "missing_key" and a location pointing to the end of the object. For some applications I would feel uncomfortable showing that error to a user that is supposed to manually edit JSON files.

Is there a way to add a more descriptive error that mentions the missing key? I know that the error context does not have much space for information like that. Maybe includer_error could be abused for this?

pfirsich avatar Jul 14 '24 06:07 pfirsich

Thanks for pointing this out.

The plan is to make the glz::context more generic, which would allow custom error messages, and I'll also take missing keys into account. We should be able to report at least one missing key. I'd like to report all missing keys, but want to be careful about avoiding dynamic allocations in the error handling code.

stephenberry avatar Jul 14 '24 17:07 stephenberry

I think we want to report all keys that are missing. This would be easiest to handle as a std::vector<std::string> of all missing keys. However, I don't generally want this within glz::context and affect performance even when error_on_missing_keys is false.

I'm thinking of having a static thread_local function that provides the missing keys. An idea for how it would be used is below:

auto ec = glz::read_json(obj, buffer);
if (ec == glz::error_code::missing_key) {
   const std::vector<std::string>& missing_keys = glz::missing_keys();
   for (auto& key : missing_keys) {
      // do something with the reported missing keys
   }
}

stephenberry avatar Aug 09 '24 20:08 stephenberry

Adding notes for future implementation: I think the best way to handle missing keys in structures is to pass around a uint64_t, where each bit indicates an index for whether the key was missing. This would only support up to 64 missing keys, but most structures aren't this large. If the structure had over 64 elements then we could use an array of uint64_t integers (glz::bitset). The real issue is that as we desire more feedback in certain cases this makes the glz::context larger. Thus far, we've avoided templating on the glz::context type. But, for performance reasons and flexibility I think Glaze needs to be carefully rearchitected to handle various context types, where additional information can be embedded based on compile time options.

I have a path forward for much better compile time option handling, I think this should be done before context changes, but done with context changes in mind.

stephenberry avatar Jan 10 '25 19:01 stephenberry

Now that we have custom_error_message as part of the output for glz::format_error, I decided that a straightforward solution is to just report the first key that is missing. I should have implemented this approach sooner, the issue is that more detailed solutions that I was originally looking at all have downsides.

Here is the new solution:

Console output for missing key "hello"

1:33: missing_key
   [{"i":287,"d":0.0,"arr":[1,2,3]}]
                                   ^ hello

The ^ points just after the object with the missing key, because we can't know any keys are missing until we have parsed the entire object.

This approach requires minimal additional executable binary and performs no heap allocations within the read_json call.

In the future this can be improved, but this is a huge step up from not getting any feedback about what key is missing.

stephenberry avatar May 30 '25 11:05 stephenberry