feat(trigger): Allow trigger to work with Lambda functions with long timeouts #23062

DerkSchooltink · 2022-11-23T16:09:55Z

Implements #23058

All Submissions:

Have you followed the guidelines in our Contributing guide?

Adding new Unconventional Dependencies:

This PR adds new unconventional dependencies following the process described here

New Features

Have you added the new feature to an integration test?
- Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

gitpod-io · 2022-11-23T16:10:01Z

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

✅ Updated pull request passes all PRLinter validations. Dissmissing previous PRLinter review.

corymhall

@DerkSchooltink thanks for the PR! Just 2 minor comments.

packages/@aws-cdk/triggers/lib/trigger.ts

packages/@aws-cdk/triggers/test/integ.triggers.ts

aws-cdk-automation · 2022-12-01T00:11:44Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

Pull request has been modified.

aws-cdk-automation · 2022-12-05T00:11:07Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

…timeouts

packages/@aws-cdk/triggers/lib/trigger.ts

Co-authored-by: Cory Hall <43035978+corymhall@users.noreply.github.com>

Pull request has been modified.

mergify · 2022-12-15T13:17:22Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

mergify · 2022-12-15T13:56:03Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

corymhall · 2022-12-16T13:13:50Z

@Mergifyio requeue

mergify · 2022-12-16T13:14:23Z

requeue

☑️ This pull request is already queued

corymhall · 2022-12-16T13:14:38Z

@Mergifyio unqueue

mergify · 2022-12-16T13:15:11Z

unqueue

✅ The pull request has been removed from the queue

corymhall · 2022-12-16T13:18:21Z

@Mergifyio requeue

mergify · 2022-12-16T13:18:53Z

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

aws-cdk-automation · 2022-12-16T16:27:56Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 5e85681
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2022-12-16T16:28:15Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

…th long timeouts (#23062)" This reverts commit 9fd3811.

…23728) Reverts #23062. #23062 introduced #23407, which causes lambda functions that use triggers to fail to invoke with either this error ``` submit response to cloudformation { Status: 'FAILED', Reason: `TypeError [ERR_INVALID_ARG_TYPE]: The "msecs" argument must be of type number. Received type string ('120000')\n` + ' at new NodeError (internal/errors.js:322:7)\n' + ' at validateNumber (internal/validators.js:129:11)\n' + ' at getTimerDuration (internal/timers.js:384:3)\n' + ' at ClientRequest.setTimeout (_http_client.js:865:11)\n' + ' at features.constructor.handleRequest (/var/runtime/node_modules/aws-sdk/lib/http/node.js:82:12)\n' + ' at executeSend (/var/runtime/node_modules/aws-sdk/lib/event_listeners.js:370:29)\n' + ' at Request.SEND (/var/runtime/node_modules/aws-sdk/lib/event_listeners.js:384:9)\n' + ' at Request.callListeners (/var/runtime/node_modules/aws-sdk/lib/sequential_executor.js:102:18)\n' + ' at Request.emit (/var/runtime/node_modules/aws-sdk/lib/sequential_executor.js:78:10)\n' + ' at Request.emit (/var/runtime/node_modules/aws-sdk/lib/request.js:686:14)', StackId: 'arn:aws:cloudformation:us-east-1:***:stack/TestStack/83f77790-806c-11ed-8956-0a55d38b49ed', RequestId: '86a74312-347d-40c4-873a-09eed5b8eddd', PhysicalResourceId: 'AWSCDK::CustomResourceProviderFramework::CREATE_FAILED', LogicalResourceId: 'Trigger', NoEcho: undefined, Data: undefined } ``` or ``` Error: Trigger handler failed with status code 202 at handler (/var/task/index.js:53:15) at processTicksAndRejections (internal/process/task_queues.js:95:5) at async Runtime.handler (/var/task/__entrypoint__.js:32:24) (RequestId: ab252a7a-e06b-4fcf-b2b4-f59b2dd88734) ``` Reverting for now, since people are unable to upgrade. To unblock this revert, I'm disabling the integration test `dependencies-pnpm`, because when running it locally (and in this PR build) the logical ID of the version changes every run. This has been reproduced by others locally, so it's being disabled until we can resolve that issue. Fixes #23407

…timeouts (aws#23062) Implements aws#23058 ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

@TheRealAmazonKendra

…timeouts (#24435) Reopened PR from #23650 and #23788 And re-revert from #23062 ([v2.61.1](https://github.com/aws/aws-cdk/releases/tag/v2.61.1)) Implements #23058 and fixes #23407 Worked on comments from @TheRealAmazonKendra as well. Added an integration test scenario where a Lambda function will be triggered by a Trigger construct, send a message to an SQS queue, which will be consumed by the integration test assertion.

github-actions bot added valued-contributor [Pilot] contributed between 6-12 PRs to the CDK p2 labels Nov 23, 2022

aws-cdk-automation requested a review from a team November 23, 2022 16:10

aws-cdk-automation previously requested changes Nov 23, 2022

View reviewed changes

corymhall previously requested changes Nov 30, 2022

View reviewed changes

packages/@aws-cdk/triggers/lib/trigger.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/triggers/test/integ.triggers.ts Show resolved Hide resolved

DerkSch added 6 commits December 5, 2022 14:41

feat(trigger): Allow trigger to work with Lambda functions with long …

b823a87

…timeouts

add documentation & update integration test

0c3b70f

add trigger consruct with optional parameters to integration test

814ec8b

add invocation type enum & add test scenario

3b72deb

Add missing changes

6ff80c8

resolve conflicts

3e97e06

DerkSchooltink force-pushed the feature/notriggertimeout branch from f47db06 to 3e97e06 Compare December 5, 2022 16:11

corymhall self-assigned this Dec 6, 2022

Merge branch 'main' into feature/notriggertimeout

f822516

corymhall previously requested changes Dec 12, 2022

View reviewed changes

packages/@aws-cdk/triggers/lib/trigger.ts Outdated Show resolved Hide resolved

apply toMillis for default timeout suggestion

5eb89d6

Co-authored-by: Cory Hall <43035978+corymhall@users.noreply.github.com>

apply duration > milliseconds change to integration test

7809c62

DerkSchooltink requested a review from corymhall December 15, 2022 09:52

corymhall approved these changes Dec 15, 2022

View reviewed changes

Merge branch 'main' into feature/notriggertimeout

45fc87c

Merge branch 'main' into feature/notriggertimeout

5e85681

mergify bot merged commit 9fd3811 into aws:main Dec 16, 2022

DerkSchooltink deleted the feature/notriggertimeout branch December 19, 2022 09:15

cecheta mentioned this pull request Dec 20, 2022

(triggers): deployments of trigger functions fail with TypeError [ERR_INVALID_ARG_TYPE]: The "msecs" argument must be of type number. Received type string #23407

Closed

comcalvi added a commit that referenced this pull request Jan 18, 2023

Revert "feat(trigger): Allow trigger to work with Lambda functions wi…

c94a999

…th long timeouts (#23062)" This reverts commit 9fd3811.

comcalvi mentioned this pull request Jan 18, 2023

fix(lambda): lambda functions that use triggers error when invoked #23728

Merged

comcalvi added a commit that referenced this pull request Jan 19, 2023

Revert "feat(trigger): Allow trigger to work with Lambda functions wi…

01e243f

…th long timeouts (#23062)" This reverts commit 9fd3811.

DerkSchooltink mentioned this pull request Jan 23, 2023

feat(trigger): Allow trigger to work with Lambda functions with long timeouts #23788

Closed

4 tasks

DerkSchooltink mentioned this pull request Mar 3, 2023

feat(trigger): Allow trigger to work with Lambda functions with long timeouts #24435

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(trigger): Allow trigger to work with Lambda functions with long timeouts #23062

feat(trigger): Allow trigger to work with Lambda functions with long timeouts #23062

DerkSchooltink commented Nov 23, 2022 •

edited

Loading

gitpod-io bot commented Nov 23, 2022

aws-cdk-automation left a comment

corymhall left a comment

aws-cdk-automation commented Dec 1, 2022

aws-cdk-automation commented Dec 5, 2022

mergify bot commented Dec 15, 2022

mergify bot commented Dec 15, 2022

corymhall commented Dec 16, 2022

mergify bot commented Dec 16, 2022

corymhall commented Dec 16, 2022

mergify bot commented Dec 16, 2022

corymhall commented Dec 16, 2022

mergify bot commented Dec 16, 2022

aws-cdk-automation commented Dec 16, 2022

mergify bot commented Dec 16, 2022

feat(trigger): Allow trigger to work with Lambda functions with long timeouts #23062

feat(trigger): Allow trigger to work with Lambda functions with long timeouts #23062

Conversation

DerkSchooltink commented Nov 23, 2022 • edited Loading

Implements #23058

All Submissions:

Adding new Unconventional Dependencies:

New Features

gitpod-io bot commented Nov 23, 2022

aws-cdk-automation left a comment

Choose a reason for hiding this comment

corymhall left a comment

Choose a reason for hiding this comment

aws-cdk-automation commented Dec 1, 2022

aws-cdk-automation commented Dec 5, 2022

mergify bot commented Dec 15, 2022

mergify bot commented Dec 15, 2022

corymhall commented Dec 16, 2022

mergify bot commented Dec 16, 2022

☑️ This pull request is already queued

corymhall commented Dec 16, 2022

mergify bot commented Dec 16, 2022

✅ The pull request has been removed from the queue

corymhall commented Dec 16, 2022

mergify bot commented Dec 16, 2022

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

aws-cdk-automation commented Dec 16, 2022

AWS CodeBuild CI Report

mergify bot commented Dec 16, 2022

DerkSchooltink commented Nov 23, 2022 •

edited

Loading