Skip to content

fix(event_handler): split OpenAPI validation to respect middleware returns #7050

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

Merged
merged 5 commits into from
Jul 30, 2025

Conversation

leandrodamascena
Copy link
Contributor

@leandrodamascena leandrodamascena commented Jul 28, 2025

Issue number: #5228
Discussion: #4656

Summary

Fixes middleware validation issues by splitting OpenAPI validation into separate request and response validation middlewares.

Changes

New Middleware Architecture

Split the OpenAPIValidationMiddleware into two separate middlewares:

  1. OpenAPIRequestValidationMiddleware - Validates incoming requests only
  2. OpenAPIResponseValidationMiddleware - Validates outgoing responses only

Execution Order

┌─────────────────────────────────────────────────────────────────┐
│                    Request Flow                                 │
├─────────────────────────────────────────────────────────────────┤
│ 1. Request Validation Middleware                                │
│    ├─ Validates path params, query params, headers, body       │
│    ├─ If invalid → Returns 422, stops execution               │
│    └─ If valid → Continues to next middleware                  │
│                                                                 │
│ 2. User Middlewares (in order of registration)                 │
│    ├─ Auth middleware                                          │
│    ├─ Rate limiting middleware                                 │
│    ├─ Logging middleware                                       │
│    ├─ etc...                                                   │
│    └─ Can return early responses (401, 403, 429, etc.)        │
│                                                                 │
│ 3. Response Validation Middleware                              │
│    ├─ Only executes if reached (no early returns)             │
│    └─ Validates response against OpenAPI schema               │
│                                                                 │
│ 4. Route Handler                                               │
│    └─ The actual endpoint function                             │
└─────────────────────────────────────────────────────────────────┘

User experience

Breaking change consideration

⚠️ Potential Impact: This fix may affect customers who were working around the previous incorrect behavior by catching RequestValidationError exceptions from middleware responses. However, this was unintended behavior that needed to be corrected.

Why this change is necessary:

  • The previous behavior was a bug - middleware responses should not be validated against OpenAPI schemas
  • Middleware responses (401, 403, 429) are control flow responses, not data responses
  • The fix aligns with expected middleware behavior patterns
  • Continuing the incorrect behavior would prevent proper middleware functionality

Checklist

If your change doesn't seem to apply, please leave them unchecked.

Is this a breaking change?

RFC issue number:

Checklist:

  • Migration process documented
  • Implement warnings (if it can live side by side)

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@leandrodamascena leandrodamascena requested a review from a team as a code owner July 28, 2025 16:09
@pull-request-size pull-request-size bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jul 28, 2025
@boring-cyborg boring-cyborg bot added documentation Improvements or additions to documentation event_handlers tests labels Jul 28, 2025
@github-actions github-actions bot added bug Something isn't working and removed documentation Improvements or additions to documentation labels Jul 28, 2025
@leandrodamascena leandrodamascena changed the title fix(event_handler): split OpenAPI validation to respect middleware early returns - WIP fix(event_handler): split OpenAPI validation to respect middleware returns - WIP Jul 28, 2025
@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Jul 28, 2025
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Jul 28, 2025
Copy link

codecov bot commented Jul 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.33%. Comparing base (c614d5b) to head (a6bbdcc).
⚠️ Report is 2 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #7050      +/-   ##
===========================================
+ Coverage    96.26%   96.33%   +0.07%     
===========================================
  Files          275      275              
  Lines        12917    12921       +4     
  Branches       955      953       -2     
===========================================
+ Hits         12435    12448      +13     
+ Misses         376      367       -9     
  Partials       106      106              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Jul 28, 2025
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Jul 28, 2025
@leandrodamascena
Copy link
Contributor Author

Hey @m-walmsley! I'd love to have your review and opinion here.

@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Jul 28, 2025
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Jul 28, 2025
@leandrodamascena leandrodamascena changed the title fix(event_handler): split OpenAPI validation to respect middleware returns - WIP fix(event_handler): split OpenAPI validation to respect middleware returns Jul 28, 2025
@leandrodamascena leandrodamascena self-assigned this Jul 28, 2025
@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Jul 28, 2025
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Jul 28, 2025
@walmsles
Copy link
Contributor

Hey @m-walmsley! I'd love to have your review and opinion here.

Sure - but that's not the real me (need to remove that one - it was for internal Accenture projects)

@leandrodamascena
Copy link
Contributor Author

Hey @m-walmsley! I'd love to have your review and opinion here.

Sure - but that's not the real me (need to remove that one - it was for internal Accenture projects)

Yes, sorry for that! It was the autocomplete.

Thanks.

@leandrodamascena
Copy link
Contributor Author

@walmsles and I met today to discuss this change and the impact it may have on customers who already use Middleware and Data Validation. I'd like to highlight a few points for clarity:

1/ Customers who use Middleware but do not use Data Validation will not be impacted.

2/ Customers who use Data Validation but do not use Middleware will not be impacted.

3/ Customers who use Middleware + Data Validation but do not use early return with Middleware will not be impacted.

4/ Customers who use Middleware + Data Validation + Early Return and use the exception handler to intercept RequestValidationError or ResponseValidationError may be impacted, as it will no longer raise these exceptions and will respect the Middleware's return. The Powertools team does not consider this a breaking change, as this is incorrect behavior and we want to provide the best customer experience. However, if there are many customers complaining about this fix, we may revert the PR. We will monitor this.

We're also looking to the future, and it makes sense for customers to have full control over the execution of the middleware stack. So, I'll open an issue to expose OpenAPIRequestValidationMiddleware and OpenAPIResponseValidationMiddleware to customers, so they can build their middleware stack however they want. We may also consider adding a new field to add priority in the order that Middleware can be executed.

@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Jul 30, 2025
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Jul 30, 2025
@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Jul 30, 2025
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Jul 30, 2025
Copy link

@leandrodamascena leandrodamascena requested review from dreamorosi and removed request for anafalcao July 30, 2025 12:16
@leandrodamascena
Copy link
Contributor Author

@leandrodamascena leandrodamascena merged commit 7f1abc4 into develop Jul 30, 2025
22 of 28 checks passed
@leandrodamascena leandrodamascena deleted the fix/avoid-middleware-validation branch July 30, 2025 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working event_handlers size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: APIGatewayRestResolver(enable_validation=True) is validating middleware responses
3 participants