Skip to content
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.

Sign up for GitHub

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

Jump to bottom

aws-cdk-lib/triggers: Confusing/missing explanation on trigger failures #26341

Closed
archevel opened this issue Jul 13, 2023 · 2 comments · Fixed by #26450
Closed

aws-cdk-lib/triggers: Confusing/missing explanation on trigger failures #26341

archevel opened this issue Jul 13, 2023 · 2 comments · Fixed by #26450
Labels
@aws-cdk/triggers Related to the triggers package bug This issue is a bug. documentation This is a problem with documentation. effort/small Small work item – less than a day of effort p2

Comments

@archevel
Copy link

Describe the issue

From the docs about trigger failures:

If the trigger handler fails (e.g. an exception is raised), the CloudFormation deployment will fail, as if a resource failed to provision.

However, when the invocationType in the trigger's props is triggers.InvocationType.EVENT a deployment will succeed regardless of if an error is thrown in the handler body. The quoted statement is true if triggers.InvocationType.REQUEST_RESPONSE is used however. Since the example code snippet of a trigger uses triggers.InvocationType.EVENT it gives the impression that the deployment will fail if an exception is raised in the handler for the event. Perhaps this is an actual bug in this case, but if it works as intended then it would be good to be explicit about the effect of the different invocationTypes in the documentation.

Links

aws-cdk-lib
@archevel archevel added documentation This is a problem with documentation. needs-triage This issue or PR still needs to be triaged. labels Jul 13, 2023
@github-actions github-actions bot added the @aws-cdk/triggers Related to the triggers package label Jul 13, 2023
@pahud
Copy link
Contributor

pahud commented Jul 13, 2023

Yes you are right. We definitely should update the doc. Any interest to submit a PR for this small doc fix?

@pahud pahud added p2 effort/small Small work item – less than a day of effort bug This issue is a bug. and removed needs-triage This issue or PR still needs to be triaged. labels Jul 13, 2023
@tam0ri tam0ri mentioned this issue Jul 20, 2023
Merged
@mergify mergify bot closed this as completed in #26450 Jul 20, 2023
mergify bot pushed a commit that referenced this issue Jul 20, 2023
@tam0ri
docs(triggers): add consideration about invocationType for the behavi…
aab3f2e 
…or of trigger failures (#26450)

TriggerCustomResourceProvider takes only the status code of Invoke API call into account.
https://github.com/aws/aws-cdk/blob/7a6f953fe5a4d7e0ba5833f06596b132c95e0709/packages/aws-cdk-lib/triggers/lib/lambda/index.ts#L69-L73

If invocationType is `EVENT`, Lambda function is invoked asynchronously. In that case, if Lambda function is invoked successfully, the trigger will success regardless of the result for the function execution. I add this consideration into README.

Closes #26341

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

bmoffatt pushed a commit to bmoffatt/aws-cdk that referenced this issue Jul 29, 2023
@tam0ri @bmoffatt
docs(triggers): add consideration about invocationType for the behavi…
d51b6e9 
…or of trigger failures (aws#26450)

TriggerCustomResourceProvider takes only the status code of Invoke API call into account.
https://github.com/aws/aws-cdk/blob/7a6f953fe5a4d7e0ba5833f06596b132c95e0709/packages/aws-cdk-lib/triggers/lib/lambda/index.ts#L69-L73

If invocationType is `EVENT`, Lambda function is invoked asynchronously. In that case, if Lambda function is invoked successfully, the trigger will success regardless of the result for the function execution. I add this consideration into README.

Closes aws#26341

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions github-actions bot mentioned this issue Aug 1, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
@aws-cdk/triggers Related to the triggers package bug This issue is a bug. documentation This is a problem with documentation. effort/small Small work item – less than a day of effort p2
Projects
None yet
Milestone
No milestone
2 participants
@archevel @pahud