Dev Merge #103
Dev Merge #103
Conversation
…agation-issue added 500 ms delay after lambda created for aws lambda trust policy propagation
WalkthroughVersion increments in core and SDK package.json files. Lambda creation helper adjusted: removed capturing/logging of CreateFunctionCommand response, added a 500ms wait after creation before verifying deployment status; update path unchanged. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant AWSLambdaCodeHelper
participant AWSLambda
Caller->>AWSLambdaCodeHelper: createOrUpdateLambda()
alt First-time create
AWSLambdaCodeHelper->>AWSLambda: CreateFunctionCommand
AWSLambda-->>AWSLambdaCodeHelper: ack
Note over AWSLambdaCodeHelper: Wait ~500ms (trust policy propagation)
AWSLambdaCodeHelper->>AWSLambda: verifyFunctionDeploymentStatus()
AWSLambda-->>AWSLambdaCodeHelper: status
else Update existing
AWSLambdaCodeHelper->>AWSLambda: UpdateFunctionCode/Config
AWSLambda-->>AWSLambdaCodeHelper: status
end
AWSLambdaCodeHelper-->>Caller: result
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/core/src/helpers/AWSLambdaCode.helper.ts (3)
246-262: Harden deployment waiter: check State + LastUpdateStatus, add timeout/backoff, handle failures.Current implementation can loop forever and ignores
State/ failure reasons.Apply this diff:
-export async function verifyFunctionDeploymentStatus(functionName, client): Promise<boolean> { - return new Promise((resolve, reject) => { - try { - let interval = setInterval(async () => { - const getFunctionCommand = new GetFunctionCommand({ FunctionName: functionName }); - const lambdaResponse = await client.send(getFunctionCommand); - - if (lambdaResponse.Configuration.LastUpdateStatus === 'Successful') { - clearInterval(interval); - return resolve(true); - } - }, 5000); - } catch (error) { - return false; - } - }); -} +export async function verifyFunctionDeploymentStatus( + functionName: string, + client: LambdaClient, + opts: { timeoutMs?: number; pollMs?: number } = {}, +): Promise<boolean> { + const timeoutAt = Date.now() + (opts.timeoutMs ?? 120_000); + const pollMs = opts.pollMs ?? 2_000; + while (Date.now() < timeoutAt) { + const lambdaResponse = await client.send(new GetFunctionCommand({ FunctionName: functionName })); + const cfg = lambdaResponse.Configuration!; + if (cfg.State === 'Active' && cfg.LastUpdateStatus === 'Successful') { + return true; + } + if (cfg.LastUpdateStatus === 'Failed' || cfg.State === 'Failed') { + const reason = cfg.LastUpdateStatusReason || cfg.StateReason || 'unknown'; + throw new Error(`Lambda ${functionName} deployment failed: ${reason}`); + } + await new Promise((r) => setTimeout(r, pollMs)); + } + throw new Error(`Timed out waiting for Lambda ${functionName} to become Active & Successful`); +}
229-244: Role waiter can hang forever; add timeout/backoff and handle transient IAM errors.
setIntervalwith async body risks overlaps and never resolves on errors.Apply this diff:
-export async function waitForRoleDeploymentStatus(roleName, client): Promise<boolean> { - return new Promise((resolve, reject) => { - try { - let interval = setInterval(async () => { - const getRoleCommand = new GetRoleCommand({ RoleName: roleName }); - const roleResponse = await client.send(getRoleCommand); - if (roleResponse.Role.AssumeRolePolicyDocument) { - clearInterval(interval); - return resolve(true); - } - }, 7000); - } catch (error) { - return false; - } - }); -} +export async function waitForRoleDeploymentStatus( + roleName: string, + client: IAMClient, + opts: { timeoutMs?: number; pollMs?: number } = {}, +): Promise<boolean> { + const timeoutAt = Date.now() + (opts.timeoutMs ?? 90_000); + const pollMs = opts.pollMs ?? 3_000; + while (Date.now() < timeoutAt) { + try { + const roleResponse = await client.send(new GetRoleCommand({ RoleName: roleName })); + if (roleResponse.Role?.AssumeRolePolicyDocument) return true; + } catch (err: any) { + if (!['NoSuchEntityException', 'ThrottlingException'].includes(err?.name)) { + throw err; + } + } + await new Promise((r) => setTimeout(r, pollMs)); + } + throw new Error(`Timed out waiting for IAM role ${roleName} assume-role policy to propagate`); +}
195-195: Upgrade Lambda runtime to Node.js 20.x.Node 18.x is EOL; prefer
Runtime.nodejs20xfor security/support.Apply this diff:
- Runtime: Runtime.nodejs18x, + Runtime: Runtime.nodejs20x,
🧹 Nitpick comments (1)
packages/core/src/helpers/AWSLambdaCode.helper.ts (1)
206-206: OK to drop capturing the CreateFunction response; optional ARN log.If ARN telemetry is useful, consider logging
FunctionArnfrom the response; otherwise this is fine.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/core/package.json(1 hunks)packages/core/src/helpers/AWSLambdaCode.helper.ts(1 hunks)packages/sdk/package.json(1 hunks)
🔇 Additional comments (2)
packages/sdk/package.json (1)
3-3: Version bump looks good.No functional impact; safe metadata change.
packages/core/package.json (1)
3-3: Version bump looks good.No functional impact; safe metadata change.
| // wait 500 ms to let the function trust policy be applied | ||
| // it will only occur when the function is created for the first time | ||
| await new Promise((resolve) => setTimeout(resolve, 500)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace fixed 500ms sleep with a robust waiter; 500ms is too short and flaky.
Trust-policy/IAM propagation often takes seconds. Fold readiness into the deployment waiter and remove the hard-coded delay.
Apply this diff to remove the brittle sleep:
- // wait 500 ms to let the function trust policy be applied
- // it will only occur when the function is created for the first time
- await new Promise((resolve) => setTimeout(resolve, 500));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // wait 500 ms to let the function trust policy be applied | |
| // it will only occur when the function is created for the first time | |
| await new Promise((resolve) => setTimeout(resolve, 500)); |
🤖 Prompt for AI Agents
In packages/core/src/helpers/AWSLambdaCode.helper.ts around lines 209 to 211,
replace the brittle fixed 500ms sleep with a robust waiter: remove the new
Promise setTimeout and instead integrate an AWS-ready waiter that polls for the
function's readiness or the IAM/trust-policy propagation (for example using AWS
SDK's waiter for functionActive or a custom loop with exponential backoff retry
checking GetFunctionConfiguration/GetPolicy until the expected role/trust-policy
is effective), fold this check into the existing deployment waiter flow, set a
sensible timeout (several seconds to minutes) and surface a clear error if the
waiter times out.
2 participants
Summary by CodeRabbit
Bug Fixes
Chores