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

Support custom error messages #149

Merged
merged 2 commits into from
Nov 15, 2023
Merged

Support custom error messages #149

merged 2 commits into from
Nov 15, 2023

Conversation

davishmcclurg
Copy link
Owner

This introduces two ways of defining custom error messages: the x-error keyword and I18n translations.

x-error is a json_schemer-specific schema keyword that allows overriding error messsages for a schema. It can either be a string, which overrides errors for all keywords (and for the schema itself), or a hash of keyword-specific messages. Interpolation of some values is supported using gsub because sprintf logs annoying warnings if all arguments aren't present in the string.

I18n translations are enabled when the i18n gem is loaded and a json_schemer translation key exists. Translation keys are based on schema $id, keyword location, meta-schema $id, and keyword. The lookup is ordered from most specific to least specific so that overrides can be applied as necessary.

Notes:

  • Both x-error and I18n use special catch-all (*) and schema (^) "keywords" to support fallbacks and schema-level errors, respectively.
  • x-error uses the x- prefix to match the future spec for unknown keywords: https://github.com/orgs/json-schema-org/discussions/329
  • The I18n check to enable translations is cached forever because I18n is slow and I didn't want it to hurt performance when it's not used.
  • I18n uses the ASCII unit separator (\x1F) because the . default doesn't work well with absolute URIs ($id and $schema).
  • I moved CLASSIC_ERROR_TYPES out of JSONSchemer::Result because that's actually where it's being defined. The Struct.new block scope doesn't hold constants like a class.

Closes: #143

This introduces two ways of defining custom error messages: the
`x-error` keyword and I18n translations.

`x-error` is a json_schemer-specific schema keyword that allows
overriding error messsages for a schema. It can either be a string,
which overrides errors for all keywords (and for the schema itself), or
a hash of keyword-specific messages. Interpolation of some values is
supported using `gsub` because `sprintf` logs annoying warnings if all
arguments aren't present in the string.

I18n translations are enabled when the `i18n` gem is loaded and a
`json_schemer` translation key exists. Translation keys are based on
schema $id, keyword location, meta-schema $id, and keyword. The lookup
is ordered from most specific to least specific so that overrides can be
applied as necessary.

Notes:

- Both `x-error` and I18n use special catch-all (`*`) and schema (`^`)
  "keywords" to support fallbacks and schema-level errors, respectively.
- `x-error` uses the `x-` prefix to match the future spec for unknown
  keywords: https://github.com/orgs/json-schema-org/discussions/329
- The I18n check to enable translations is cached forever because I18n
  is slow and I didn't want it to hurt performance when it's not used.
- I18n uses the ASCII unit separator (\x1F) because the `.` default
  doesn't work well with absolute URIs ($id and $schema).
- I moved `CLASSIC_ERROR_TYPES` out of `JSONSchemer::Result` because
  that's actually where it's being defined. The `Struct.new` block scope
  doesn't hold constants like a class.

Closes: #143
@davishmcclurg
Copy link
Owner Author

@ahx any interest in trying this out? I'd like to get some feedback before merging.

@ahx
Copy link
Contributor

ahx commented Oct 18, 2023

@ahx any interest in trying this out? I'd like to get some feedback before merging.

I will try to look into this this weekend latest. Don't hesitate to punch me if I don't get back to you.

@babelfish
Copy link

One suggestion I have is, in the detailed results, add some additional key to indicate that the error was set via x-error. In my use case this would be useful when, for example, setting an x-error for a composition keyword. When collecting all error messages after error validation in order to add them to the response, if I see this key indicating the error is a custom one I can stop descending down that branch of the error tree with the assumption that that custom error summarizes any errors in that branch.

These keys make it possible to determine how error messages were
generated.

#149 (comment)
@davishmcclurg
Copy link
Owner Author

One suggestion I have is, in the detailed results, add some additional key to indicate that the error was set via x-error.

@babelfish good idea—I pushed a commit that adds "x-error": true to messages generated using x-error (and "i18n": true for I18n).

@ahx
Copy link
Contributor

ahx commented Oct 24, 2023

Something not related to this PR, but to the 2.1.0 branch: I noticed that format: date is not supported for the Openapi3.0 schema. But I think it should . Or shouldn't it?

davishmcclurg added a commit that referenced this pull request Oct 24, 2023
This shouldn't've been removed in: ccffac7
I was under the impression that it would be provided by draft4 since
that's openapi30's meta schema, but it's not in that spec.

OpenAPI 3.0 formats listed here: https://spec.openapis.org/oas/v3.0.3#data-types

@ahx noticed this here: #149 (comment)
@davishmcclurg
Copy link
Owner Author

I noticed that format: date is not supported for the Openapi3.0 schema. But I think it should . Or shouldn't it?

Good catch—thanks @ahx! Should be fixed here: 69fe7a8

It's confusing that OpenAPI 3.0 is based on draft5, which isn't really a thing.

@davishmcclurg
Copy link
Owner Author

@ahx have you had a chance to look at this at all?

@ahx
Copy link
Contributor

ahx commented Oct 27, 2023

I tried out the branch in openapi_first and found out about the issue with the date format, but I have not tried the i18n integration. But I really like the idea.

@davishmcclurg davishmcclurg merged commit 257e9b9 into 2.1.0 Nov 15, 2023
62 checks passed
@davishmcclurg davishmcclurg deleted the errors branch November 15, 2023 20:10
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.

None yet

3 participants