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

Add errorcontext to unmarshalString() #856

Closed
wants to merge 1 commit into from

Conversation

Krishks369
Copy link

@Krishks369 Krishks369 commented Mar 4, 2023

This PR tries to solve the issue here

@pellitier I have added the error context to the function according to my knowledge and please do check if its correct. Also can you provide a way to test it if its correct. Thanks for the reply

ctx := d.errorContext
if ctx != nil && ctx.Struct != nil {
f := ctx.Struct.FieldByIndex(ctx.Field)
return unstable.NewParserErrorWithContext(d.p.Raw(value.Raw), "cannot store TOML string into struct field %s.%s of type %s", f.Name, f.Type, v.Kind())
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless I missed something, you should be able to use d.typeMismatchError() to generate an error that resulted from a mismatch between the expected Go type and the type found in the TOML document. It will take care of creating the human readable error based on whether the decoder is targeting a struct.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @pelletier,

Pardon my ignorance. I'm a newbie. Is there a way to get a human-readable error string when the type of a field mismatches? d.typeMismatchError() is internal, and I can't use it in my code. And, DecodeError.String() is multi-line, which is not something I want.

This is what I'm trying:
If my config looks like this,

[server]
path = "/my/path"
port = 50

and, I read it like this,

if err := toml.NewDecoder(file).Decode(&cfg); err != nil {
    return Cfg{}, fmt.Errorf("load toml: %w", err)
}

I'm getting the error message load toml: cannot store TOML string into a Go int when the port is set to a string. How can I get the error message to mention the field server.port that is causing the problem?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arkn98 sorry I didn't see this. You're doing it right; the error message should contain the name of the field causing problem. Seems like this specific error is not constructed properly to include it.

Copy link

@arkn98 arkn98 Aug 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries at all. Your fix seems to work. Let me test it out. Thanks for looking into this!

@pelletier
Copy link
Owner

@Krishks369 @arkn98 This should now be fixed by #891. Feel free to reopen this PR or create an issue if not!

@pelletier pelletier closed this Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants