Skip to content

APIGW NG implement AWS bug for INTEGRATION_FAILURE GatewayResponse #11183

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 3 commits into from
Jul 12, 2024

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented Jul 11, 2024

Motivation

While implementing the AWS_PROXY integration with #11167, I've stumbled onto a weird issue: AWS would raise an INTEGRATION_FAILURE error, which the default status code should be 504 (Gateway Timeout, that's already suspicious), but instead returned 500.

After a lot of investigation with @cloutierMat, we came to the conclusion that the CRUD later (PutGatewayResponse, GetGatewayResponse etc) is not in line with the actual behavior, and that the actual default status code is 500, but the CRUD calls still returns 504.

This PR implements the logic to enable the difference between the CRUD layer and the actual invocation layer by attaching a default status code to the exception, while still using the previous default for other exceptions, allowing us to override the behavior.

This behavior is validated by tests.aws.services.apigateway.test_apigateway_lambda.test_lambda_aws_proxy_integration_non_post_method.

Changes

  • use the Python exception status code if provided
  • add a fix in the provider to deal with the inconsistency between what the AWS CRUD operations returns and what AWS actually does under the hood

@bentsku bentsku added aws:apigateway Amazon API Gateway semver: patch Non-breaking changes which can be included in patch releases labels Jul 11, 2024
@bentsku bentsku self-assigned this Jul 11, 2024
Copy link

github-actions bot commented Jul 11, 2024

LocalStack Community integration with Pro

  2 files  ±    0    2 suites  ±0   23m 59s ⏱️ - 1h 10m 12s
662 tests  - 2 517  617 ✅  - 2 167  45 💤  - 350  0 ❌ ±0 
664 runs   - 2 517  617 ✅  - 2 167  47 💤  - 350  0 ❌ ±0 

Results for commit f32423a. ± Comparison against base commit b58b0f8.

This pull request removes 2517 tests.
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

♻️ This comment has been updated with latest results.

@bentsku bentsku marked this pull request as ready for review July 11, 2024 20:23
@bentsku bentsku requested a review from cloutierMat as a code owner July 11, 2024 20:23
Copy link
Contributor

@cloutierMat cloutierMat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀
I am glad we had a couple brains that day to figure this one out! 🤣

@bentsku bentsku merged commit d343815 into master Jul 12, 2024
32 checks passed
@bentsku bentsku deleted the apigw-ng-gateway-response branch July 12, 2024 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws:apigateway Amazon API Gateway semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants