Skip to content

fix(service): wrap schema validation sentinel via Err so errors.Is returns 422#28

Merged
alexgarzao merged 2 commits into
developfrom
fix/provider-config-validation-error-status
May 15, 2026
Merged

fix(service): wrap schema validation sentinel via Err so errors.Is returns 422#28
alexgarzao merged 2 commits into
developfrom
fix/provider-config-validation-error-status

Conversation

@alexgarzao
Copy link
Copy Markdown
Collaborator

Summary

Fixes a latent bug where POSTing (or PUTting) a provider configuration with an invalid config payload returned HTTP 500 internal server error instead of HTTP 422 Unprocessable Entity with code FLK-0293. The user saw a generic crash when in fact they sent malformed input.

Root cause

internal/services/command/create_provider_config.go (and update_provider_config.go) wrapped schema validation failures like this:

return nil, pkg.ValidationError{
    Code:    constant.ErrProviderConfigInvalidSchema.Error(), // string "FLK-0293"
    Message: fmt.Sprintf("..."),
    // Err: NOT SET — broken Unwrap chain
}

The handler at provider_configuration/handler.go:364 checks via errors.Is(err, constant.ErrProviderConfigInvalidSchema). But:

  1. pkg.ValidationError.Is() only returns true for ValidationError targets with matching Code — it cannot match plain *errors.errorString sentinels.
  2. ValidationError.Unwrap() returns e.Err, which was nil.
  3. errors.Is therefore walked an empty chain and returned false, falling through to the default 500 branch.

Fix

Set Err: constant.ErrProviderConfigInvalidSchema on both ValidationError constructions. This wires the Unwrap chain so errors.Is finds the sentinel. Principled, single-point fix — no handler changes needed.

Test plan

  • RED phase verified: regression test TestCreateProviderConfig_SchemaValidationFailure_IsConstantErrProviderConfigInvalidSchema (and the Update equivalent) fails on develop with errors.Is returning false.
  • GREEN phase: both regression tests pass after the fix.
  • Handler-layer regression: 2 new tests assert HTTP 422 + FLK-0293 body on POST and PUT with invalid config.
  • Full unit suite: 1359 tests pass across 51 packages, no regressions.
  • make lint: zero new findings (the 2 inherited pre-existing warnings on InitServers and token_fetcher.go are confirmed present on develop).
  • Integration tests not executed (require live MongoDB/HTTP infra). No integration assertion was watching for the broken 500 on this path.
  • Manual smoke: POST /v1/provider-configurations with a config missing required fields → expect 422 with FLK-0293.

Out of scope — follow-up candidates

The agent that fixed this catalogued 20+ other sites in the codebase using the same pkg.ValidationError{Code: <sentinel>.Error(), ...} pattern without setting Err. Most are in command/repository layers and may or may not have a downstream errors.Is(err, <sentinel>) check that's silently broken in the same way. A separate audit PR should:

  1. List every errors.Is(err, sentinel) check in HTTP handlers and worker dispatch paths.
  2. For each, trace back to its construction site.
  3. Ensure Err carries the sentinel wherever a domain error wraps one.

Locations flagged for future investigation:

  • activate_workflow.go:146, 253, 261, 268
  • create_workflow.go:158, 165, 183
  • execution_repository.go:69, 77, 85, 93, 103, 113
  • provider_config_repository.go:84, 91
  • get_provider_config.go:57
  • mongodb/executor_configuration/repository.go:194, 206
  • mongodb/execution/repository.go:345, 354
  • mongodb/workflow/repository.go:193, 205
  • mongodb/provider_configuration/repository.go:197, 209
  • bootstrap/tls_enforcement.go:52, 69

Plus the lookup table at pkg/errors.go:318+ — those are pre-loaded definitions and warrant separate semantic analysis before bulk-editing.

Notes

….Is works

When a client POSTs (or PATCHes) a provider configuration whose Config does
not satisfy the provider's JSON Schema, the create/update command returned
a pkg.ValidationError that carried the FLK-0293 sentinel only as a string
in Code. The handler's switch uses errors.Is on the sentinel; since
ValidationError.Is matches on Code-and-type and Unwrap returned nil, the
chain was empty and the request fell through to the default 500 branch.

Set the Err field to constant.ErrProviderConfigInvalidSchema so the
Unwrap chain reaches the sentinel and the handler maps the failure to
HTTP 422 with code FLK-0293 as documented.
…g to 422

Adds command-layer and handler-layer tests that lock in the FLK-0293
mapping for invalid provider-configuration payloads:

- create_provider_config_test.go and update_provider_config_test.go drive
  the create/update commands with a config that violates the provider
  JSON Schema and assert that the returned error satisfies both
  errors.Is(err, constant.ErrProviderConfigInvalidSchema) and
  errors.As to pkg.ValidationError with the FLK-0293 code.

- handler_test.go covers POST and PATCH /v1/provider-configurations to
  confirm the HTTP response is 422 Unprocessable Entity with code FLK-0293
  whenever the command service surfaces the wrapped sentinel.

Without the Err-field wrapping introduced in the previous commit these
tests fail (the command-layer assertions report "Should be true" on
errors.Is and the handler returns 500), guarding against any regression
that strips Err in the future.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 15, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: b24e2407-e2ef-473f-8cab-2006e8c4a19c

📥 Commits

Reviewing files that changed from the base of the PR and between 1ab9715 and 3a1a2ec.

📒 Files selected for processing (5)
  • internal/adapters/http/in/provider_configuration/handler_test.go
  • internal/services/command/create_provider_config.go
  • internal/services/command/create_provider_config_test.go
  • internal/services/command/update_provider_config.go
  • internal/services/command/update_provider_config_test.go

Walkthrough

This PR adds error-chain semantics for provider configuration schema validation failures. Create and update commands now wrap validation errors with constant.ErrProviderConfigInvalidSchema in the Err field, enabling the HTTP handler to use errors.Is() matching to return HTTP 422 instead of 500. Service-level and handler-level regression tests verify the behavior.

Changes

Provider Config Schema Validation Error Chain

Layer / File(s) Summary
Create command schema validation error wrapping
internal/services/command/create_provider_config.go, internal/services/command/create_provider_config_test.go
Create command wraps schema validation errors by setting the Err field of pkg.ValidationError to constant.ErrProviderConfigInvalidSchema. Service-level test uses a minimal test provider with required schema fields and a catalog helper to verify the error matches via errors.Is() and carries the correct validation code.
Update command schema validation error wrapping
internal/services/command/update_provider_config.go, internal/services/command/update_provider_config_test.go
Update command wraps schema validation errors identically. Service-level test constructs an update scenario with a persisted config and verifies the error chain matches and validation code is correct.
HTTP handler 422 response integration tests
internal/adapters/http/in/provider_configuration/handler_test.go
Handler regression tests assert that both create and update endpoints translate the wrapped validation error into HTTP 422 Unprocessable Entity with api.ErrorResponse code matching the sentinel and title set correctly.

Comment @coderabbitai help to get the list of available commands and usage tips.

@lerian-studio
Copy link
Copy Markdown
Contributor

📊 Unit Test Coverage Report: flowker

Metric Value
Overall Coverage 41.3% ⚠️ BELOW THRESHOLD
Threshold 85%

Coverage by Package

Package Coverage
github.com/LerianStudio/flowker/api 100.0%
github.com/LerianStudio/flowker/internal/adapters/http/in/audit 0.0%
github.com/LerianStudio/flowker/internal/adapters/http/in/catalog 60.9%
github.com/LerianStudio/flowker/internal/adapters/http/in/dashboard 91.5%
github.com/LerianStudio/flowker/internal/adapters/http/in/execution 81.0%
github.com/LerianStudio/flowker/internal/adapters/http/in/executor_configuration 72.0%
github.com/LerianStudio/flowker/internal/adapters/http/in/health 100.0%
github.com/LerianStudio/flowker/internal/adapters/http/in/middleware/testutil 0.0%
github.com/LerianStudio/flowker/internal/adapters/http/in/middleware 44.4%
github.com/LerianStudio/flowker/internal/adapters/http/in/provider_configuration 33.4%
github.com/LerianStudio/flowker/internal/adapters/http/in/readyz 81.3%
github.com/LerianStudio/flowker/internal/adapters/http/in/webhook 87.5%
github.com/LerianStudio/flowker/internal/adapters/http/in/workflow 69.7%
github.com/LerianStudio/flowker/internal/adapters/http/in 33.3%
github.com/LerianStudio/flowker/internal/adapters/mongodb/dashboard 43.0%
github.com/LerianStudio/flowker/internal/adapters/mongodb/execution 64.8%
github.com/LerianStudio/flowker/internal/adapters/mongodb/executor_configuration 36.6%
github.com/LerianStudio/flowker/internal/adapters/mongodb/provider_configuration 45.8%
github.com/LerianStudio/flowker/internal/adapters/mongodb/workflow 69.2%
github.com/LerianStudio/flowker/internal/adapters/postgresql/audit 42.2%
github.com/LerianStudio/flowker/internal/bootstrap 39.8%
github.com/LerianStudio/flowker/internal/services/command 29.9%
github.com/LerianStudio/flowker/internal/services/query 11.2%
github.com/LerianStudio/flowker/internal/services 3.2%
github.com/LerianStudio/flowker/internal/testutil 63.6%
github.com/LerianStudio/flowker/pkg/circuitbreaker 97.6%
github.com/LerianStudio/flowker/pkg/clock 0.0%
github.com/LerianStudio/flowker/pkg/condition 87.9%
github.com/LerianStudio/flowker/pkg/contextutil 0.0%
github.com/LerianStudio/flowker/pkg/executor/base 20.0%
github.com/LerianStudio/flowker/pkg/executor 39.5%
github.com/LerianStudio/flowker/pkg/executors/http/auth 59.5%
github.com/LerianStudio/flowker/pkg/executors/http 0.0%
github.com/LerianStudio/flowker/pkg/executors/midaz 76.4%
github.com/LerianStudio/flowker/pkg/executors/s3 0.0%
github.com/LerianStudio/flowker/pkg/executors/tracer 72.2%
github.com/LerianStudio/flowker/pkg/executors 57.1%
github.com/LerianStudio/flowker/pkg/model 57.7%
github.com/LerianStudio/flowker/pkg/net/http 91.3%
github.com/LerianStudio/flowker/pkg/pagination 0.0%
github.com/LerianStudio/flowker/pkg/templates/tracer_midaz 81.5%
github.com/LerianStudio/flowker/pkg/templates 0.0%
github.com/LerianStudio/flowker/pkg/transformation 67.2%
github.com/LerianStudio/flowker/pkg/triggers/webhook 0.0%
github.com/LerianStudio/flowker/pkg/triggers 0.0%
github.com/LerianStudio/flowker/pkg/webhook 100.0%
github.com/LerianStudio/flowker/pkg 90.3%

Generated by Go PR Analysis workflow

@lerian-studio
Copy link
Copy Markdown
Contributor

🔒 Security Scan Results — flowker

Trivy

Filesystem Scan

✅ No vulnerabilities or secrets found.

Docker Image Scan

✅ No vulnerabilities found.


Docker Hub Health Score Compliance

✅ Policies — 4/4 met

Policy Status
Default non-root user ✅ Passed
No fixable critical/high CVEs ✅ Passed
No high-profile vulnerabilities ✅ Passed
No AGPL v3 licenses ✅ Passed

Pre-release Version Check

✅ No unstable version pins found.


🔍 View full scan logs

@alexgarzao alexgarzao changed the title fix(command): wrap schema validation sentinel via Err so errors.Is returns 422 fix(service): wrap schema validation sentinel via Err so errors.Is returns 422 May 15, 2026
alexgarzao added a commit that referenced this pull request May 15, 2026
The Postgres testcontainer in both tests/integration and tests/e2e used
`wait.ForListeningPort("5432/tcp")`, which only checks the TCP port. Postgres
opens the port early during init but is not yet ready to accept connections,
so the first `bootstrap.AuditDatabaseManager.Connect` ping races and gets
`connection reset by peer` from the still-initializing server. This has been
flaking integration and e2e CI on develop for at least a week (visible on
runs since 2026-05-08).

Canonical fix: Postgres logs "database system is ready to accept connections"
twice during startup — once for the transient init server, once for the real
external server. Waiting for occurrence #2 (combined with the port check)
guarantees the DB will not reject incoming connections.

Scope note: this fix lives on the Makefile pre-push branch because the
Integration Tests job is part of the same CI surface and was blocking this
PR's merge despite being a pre-existing develop bug. Other open PRs (#23,
#28) will also benefit once they rebase onto a develop that includes this.
@alexgarzao alexgarzao merged commit b188a83 into develop May 15, 2026
41 of 44 checks passed
alexgarzao added a commit that referenced this pull request May 21, 2026
The Postgres testcontainer in both tests/integration and tests/e2e used
`wait.ForListeningPort("5432/tcp")`, which only checks the TCP port. Postgres
opens the port early during init but is not yet ready to accept connections,
so the first `bootstrap.AuditDatabaseManager.Connect` ping races and gets
`connection reset by peer` from the still-initializing server. This has been
flaking integration and e2e CI on develop for at least a week (visible on
runs since 2026-05-08).

Canonical fix: Postgres logs "database system is ready to accept connections"
twice during startup — once for the transient init server, once for the real
external server. Waiting for occurrence #2 (combined with the port check)
guarantees the DB will not reject incoming connections.

Scope note: this fix lives on the Makefile pre-push branch because the
Integration Tests job is part of the same CI surface and was blocking this
PR's merge despite being a pre-existing develop bug. Other open PRs (#23,
#28) will also benefit once they rebase onto a develop that includes this.
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.

2 participants