[PP-7606] Add error codes to 422 responses#4052
Open
AgaDufrat wants to merge 12 commits into
Open
Conversation
GovukStatsd has been deprecated 3 years ago and doesn't do anything [^1]. We are not monitoring the logs for the error messages. If the choose to prioritise investigation those errors we can integrate with Prometheus. [^1]: alphagov/govuk_app_config@71f4f2f
"on base_path" > "is required" test was removed because there’s no such validation. The test fails as expected but with “Base path is not a valid absolute URL path” error message which is already captured by another test
Rails validations already produce human-readable validation errors, but we also want stable machine-readable error codes for APIs consumers. We're storing codes in errors.details. Mapping is used to add the error codes to built-in validators. Using ||= ensures explicitly defined codes are preserved while still providing a fallback :validation_failed code for unmapped errors.
Rails validations already produce human-readable validation errors, but we also want stable machine-readable error codes for APIs consumers. We're storing codes in errors.details. Mapping is used to add the error codes to built-in validators. Using ||= ensures explicitly defined codes are preserved while still providing a fallback :validation_failed code for unmapped errors. Both .create! and .update! call .save! So it’s sufficient to only override .save! Methods like update_column, insert, update or bulk operations (insert_all, upsert_all) don’t call .save! but they bypass validations entirely.
We rescue the validation error so we can add our custom error codes raised by the built-in validations before raising the error again. Both .create! and .update! call .save! So it’s sufficient to only override .save! Methods like update_column, insert, update or bulk operations (insert_all, upsert_all) don’t call .save! but they bypass validations entirely. ||= ensures we don’t override existing codes with the :validation_failed fallback.
Adds support for `error_code` to `BaseCommand` and `CommandError` so we can return consistent, machine-readable error identifiers alongside HTTP errors and validation failures. This makes it easier for API clients to reliably understand and handle different failure types instead of relying on raw error messages. The argument names are quite unfortunate as `status` would probably be more appropriate for `code` but renaming it would be a breaking change, which we’re avoiding. It uses the double splat operator (**) to merge a hash into another hash. Individual error codes will be included in `fields`.
API users reported that the error message was clear and could benefit from expanding.
AgaDufrat
force-pushed
the
422-error-codes-2
branch
from
c52951b to
e3565c6
Compare
:content_store_validation_failed was chosen as currently the only GDS API adapters errors are coming from Content Store. This may need to be conditionally selected, if there are other “upstream” API errors in the future. In the Action model, while there’s only one explicit validation, associations are also validated implicitly (e.g. event must exist).
We add these checks to make sure every 422 validation error has a clear, predefined error code. This keeps API errors consistent and prevents missing or invalid error codes from slipping through. The constant can also act as lightweight documentation of supported API error codes. In development and test we raise errors immediately so missing codes are caught early and fixed during implementation. In production we notify Sentry instead of raising, which gives visibility into gaps without risking unnecessary failures for end users (500 errors instead of 422). This is a pragmatic approach because not every validation error is surfaced through the API, so requiring exhaustive error codes everywhere would add maintenance overhead.
AgaDufrat
force-pushed
the
422-error-codes-2
branch
from
e3565c6 to
7714cde
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes proposed
error_codeby adding support for it inBaseCommandandCommandErrorActiveRecord::RecordInvalid) include codes inerrors.details:validation_failedin relevant modelsCommandErrorvalidates that 422 errors always include a validerror_codeand only predefined error codes are allowed (which acts as lightweight documentation of possible codes)ArgumentErrorin test/development This avoidsWhy
Publishing applications (e.g. Manuals Publisher) currently rely on regex parsing of the human-readable message field returned by Publishing API to determine the underlying validation failure to then display it nicely to the users in the UI.
Providing stable validation error codes will:
Notes
GovukStatsdusage from CommandError and BaseCommandActiveRecordErrorstore the values undercodekey, while higher level classes and response useerror_code. We may want to keep it consistent.