Skip to content

StepFunctions: Fix ErrorName Value for Lambda Task Errors #11957

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 1 commit into from
Nov 29, 2024

Conversation

MEPalma
Copy link
Contributor

@MEPalma MEPalma commented Nov 28, 2024

Motivation

Currently, the SFN v2 interpreter incorrectly assigns the error name "Exception" to Lambda task failures that result in a LambdaFunctionErrorException, rather than utilising the exception's errorType value. This limitation prevents users from catching or retrying Lambda task failures using custom exception values, as reported in this issue. These changes address the error name computation, correcting the behaviour, and introduce relevant tests for both legacy Lambda invocations and service task Lambda invocations.

Changes

  • revised error name computation for legacy task lambda states
  • revised error name computation for service task lambda states
  • added related snapshot tests

@MEPalma MEPalma added the semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases label Nov 28, 2024
@MEPalma MEPalma self-assigned this Nov 28, 2024
@MEPalma MEPalma added semver: patch Non-breaking changes which can be included in patch releases and removed semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases labels Nov 28, 2024
Copy link

LocalStack Community integration with Pro

    2 files  ±    0      2 suites  ±0   34m 58s ⏱️ - 1h 16m 1s
1 266 tests  - 2 509  1 173 ✅  - 2 256  93 💤  - 253  0 ❌ ±0 
1 268 runs   - 2 509  1 173 ✅  - 2 256  95 💤  - 253  0 ❌ ±0 

Results for commit 70b8455. ± Comparison against base commit 892eb4d.

This pull request removes 2511 and adds 2 tests. Note that renamed tests count towards both.
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]
…
tests.aws.services.stepfunctions.v2.error_handling.test_task_lambda.TestTaskLambda ‑ test_raise_custom_exception
tests.aws.services.stepfunctions.v2.error_handling.test_task_service_lambda.TestTaskServiceLambda ‑ test_raise_custom_exception

@MEPalma MEPalma added this to the 4.0.3 milestone Nov 29, 2024
Copy link
Contributor

@gregfurman gregfurman left a comment

Choose a reason for hiding this comment

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

LGTM! Nothing that can't be done in a follow-uo

@@ -53,6 +48,40 @@ def test_raise_exception(
exec_input,
)

@markers.aws.validated
def test_raise_custom_exception(
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a JSONata test to this as well in a follow-up? Curious if the $states.errorOutput works here too.

Comment on lines +66 to +78
if isinstance(ex, ClientError):
error_name = CustomErrorName(error)
cause = ex.response["Error"]["Message"]
elif isinstance(ex, lambda_eval_utils.LambdaFunctionErrorException):
cause = ex.payload
elif isinstance(ex, ClientError):
try:
cause_object = json.loads(cause)
error = cause_object["errorType"]
except Exception as ex:
LOG.warning(
"Could not retrieve 'errorType' field from LambdaFunctionErrorException object: %s",
ex,
)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Could be worth unifying this function and the above to prevent duplication in both the StateTaskLambda and StateTaskServiceLambda

@MEPalma MEPalma merged commit 6bc1e1e into master Nov 29, 2024
40 of 42 checks passed
@MEPalma MEPalma deleted the MEP-sfn-lambda_custom_ex branch November 29, 2024 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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