Skip to content

Conversation

@rledisez
Copy link
Contributor

Currently when a min/max value for a number field is not respected, the api only returns a generic error message without providing any useful information to fix the request. This commit add the field name to the error message to help the user of the API.

@rledisez
Copy link
Contributor Author

Try to be more user friendly on error messages

Any feedbacks appreciated @antihax @grokify @kemokemo @jirikuncar @ph4r5h4d @lwj5

@lwj5
Copy link
Contributor

lwj5 commented Apr 30, 2024

It would be better if the errors were wrapped, instead of passing the name into the function.

fmt.Errorf(“param name: %w” err)

there’s also nested ParsingError which we might want to remove. I think the error handling in this quite old before the error wrapping came out

Currently when a parsing rule of a field is not respected (eg: min/max
value, required, ...), the api only returns an  error message without
providing the field name to help the user to fix the request. This commit
add the field name to the error message to help the user of the API.
@rledisez rledisez force-pushed the goServerMinMaxErrMsg branch from b648053 to bf0fe55 Compare April 30, 2024 09:00
@rledisez
Copy link
Contributor Author

Actually, I changed a bit the way to do it to add a Param field to ErrParsing so it can apply to all parsing error (eg: WithRequire, WithParse, ...). So the original error is kept. Let me know if it looks better to you.

@rledisez rledisez changed the title [go-server] add field name in min/max error messages [go-server] add field name in parsing error messages Apr 30, 2024
@lwj5
Copy link
Contributor

lwj5 commented Apr 30, 2024

Yes it looks good. Let me take a deeper look and approve it tmr

@lwj5
Copy link
Contributor

lwj5 commented May 1, 2024

Lgtm. I’ll update the error handler with errors.As() to ensure errors are handled correctly in another PR

@wing328 wing328 merged commit 230e8ce into OpenAPITools:master May 1, 2024
@wing328 wing328 added this to the 7.6.0 milestone May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants