Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance missing_key error message #1148

Open
pfirsich opened this issue Jul 14, 2024 · 3 comments
Open

Enhance missing_key error message #1148

pfirsich opened this issue Jul 14, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@pfirsich
Copy link

pfirsich commented Jul 14, 2024

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?

@stephenberry
Copy link
Owner

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
Copy link
Owner

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
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants