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.

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

cdk synth (v1.75.0) throws error with aws-lambda-step-function #108

Closed
knihit opened this issue Dec 9, 2020 · 7 comments
Closed

cdk synth (v1.75.0) throws error with aws-lambda-step-function #108

knihit opened this issue Dec 9, 2020 · 7 comments
Assignees
Labels
bug Something isn't working

Comments

@knihit
Copy link
Member

knihit commented Dec 9, 2020

When synthesizing a construct that uses aws-lambda-step-function, an error is thrown and synthesis fails. When looking at the stack trace, the root cause seems to be linked to the override warning that is published during synthesis. I can confirm that there were no issues until v1.73.0.

Reproduction Steps

A simple code to invoke the construct

            const logGroup = new LogGroup(scope, 'StMacLogGroup');
            const lambdaStepFunction = new LambdaToStepFunction(this, 'WorkflowEngine', {
                existingLambdaObj: props.lambdaFunc,
                stateMachineProps: {
                    definition: props.chain,
                    ...(props.stateMachineType === StateMachineType.EXPRESS && {
                        stateMachineType: props.stateMachineType
                    }),
                    logs: {
                        destination: logGroup,
                        level: LogLevel.FATAL,
                        includeExecutionData: false
                    }
                }
            });

Error Log

Argument to Intrinsic must be a plain value object, got () => {
            throw new Error('Duration.toString() was used, but .toSeconds, .toMinutes or .toDays should have been called instead');
        }
Subprocess exited with error 1

Looking at the stack strace, the error seems to be related to the following lines from core/lib

      at findOverrides (node_modules/@aws-solutions-constructs/core/lib/override-warning-service.ts:62:25)
      at Object.flagOverriddenDefaults (node_modules/@aws-solutions-constructs/core/lib/override-warning-service.ts:24:21)

I suppressed the warning message using
export overrideWarningsEnabled=false
and the synthesis is successful.

Environment

  • **CDK CLI Version :1.76.0
  • **CDK Framework Version:1.75.0
  • **AWS Solutions Constructs Version :1.75.0
  • **OS :MacOS
  • **Language :Typescript

Other

Full stack trace when running a unit test

Argument to Intrinsic must be a plain value object, got () => {
                throw new Error('Duration.toString() was used, but .toSeconds, .toMinutes or .toDays should have been called instead');
            }

      38 | 
      39 |         if (props.lambdaFunc != null && props.lambdaFunc != undefined) {
    > 40 |             const lambdaStepFunction = new LambdaToStepFunction(this, 'WorkflowEngine', {
         |                                        ^
      41 |                 existingLambdaObj: props.lambdaFunc,
      42 |                 stateMachineProps: {
      43 |                     definition: props.chain,

      at new Intrinsic (node_modules/@aws-cdk/core/lib/private/intrinsic.ts:39:13)
      at Function.asAny (node_modules/@aws-cdk/core/lib/token.ts:102:48)
      at Function.asString (node_modules/@aws-cdk/core/lib/token.ts:79:53)
      at Duration.toString (node_modules/@aws-cdk/core/lib/duration.ts:219:18)
      at realTypeOf (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:117:81)
      at deepDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:193:9)
      at deepDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:255:15)
      at deepDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:255:15)
      at deepDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:255:15)
      at deepDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:255:15)
      at deepDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:255:15)
      at deepDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:255:15)
      at deepDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:255:15)
      at deepDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:255:15)
      at deepDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:255:15)
      at deepDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:255:15)
      at deepDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:255:15)
      at deepDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:255:15)
      at deepDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:255:15)
      at deepDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:255:15)
      at deepDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:255:15)
      at deepDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:255:15)
      at deepDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:255:15)
      at deepDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:255:15)
      at deepDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:255:15)
      at deepDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:255:15)
      at deepDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:255:15)
      at deepDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:255:15)
      at deepDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:255:15)
      at deepDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:255:15)
      at deepDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:255:15)
      at observableDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:282:5)
      at Function.accumulateDiff (node_modules/@aws-solutions-constructs/core/node_modules/deep-diff/index.js:302:19)
      at findOverrides (node_modules/@aws-solutions-constructs/core/lib/override-warning-service.ts:62:25)
      at Object.flagOverriddenDefaults (node_modules/@aws-solutions-constructs/core/lib/override-warning-service.ts:24:21)
      at Object.overrideProps (node_modules/@aws-solutions-constructs/core/lib/utils.ts:66:5)
      at Object.buildStateMachine (node_modules/@aws-solutions-constructs/core/lib/step-function-helper.ts:41:22)
      at new LambdaToStepFunction (node_modules/@aws-solutions-constructs/aws-lambda-step-function/lib/index.ts:73:65)
      at new Workflow (lib/text-analysis-workflow/workflow-construct.ts:40:40)
      at new TextOrchestration (lib/text-analysis-workflow/text-orchestration-construct.ts:277:30)
      at new DiscoveringHotTopicsStack (lib/discovering-hot-topics-stack.ts:176:36)
      at Object.<anonymous> (test/discovering-hot-topics-stack.test.ts:23:5)

This is 🐛 Bug Report

@knihit knihit added bug Something isn't working needs-triage The issue or PR still needs to be triaged labels Dec 9, 2020
@hayesry
Copy link
Contributor

hayesry commented Dec 14, 2020

Thanks @knihit, looking into it!

@hnishar hnishar added in-progress This issue is being actively worked on and removed needs-triage The issue or PR still needs to be triaged labels Dec 14, 2020
@hayesry
Copy link
Contributor

hayesry commented Dec 21, 2020

@knihit Thanks for your patience. I tried recreating your code using a variant of the integ.existing-function.ts integration test file, and running it on v1.73.0, v1.75.0, and v1.77.0 (the latest). I wasn't able to reproduce the error using the following setup:

// Imports
import { App, Stack } from "@aws-cdk/core";
import { LambdaToStepFunction, LambdaToStepFunctionProps } from "../lib";
import * as lambda from '@aws-cdk/aws-lambda';
import * as stepfunctions from '@aws-cdk/aws-stepfunctions';
import * as defaults from '@aws-solutions-constructs/core';
import { LogGroup } from '@aws-cdk/aws-logs';

// Setup the app and stack
const app = new App();
const stack = new Stack(app, 'test-lambda-step-function-stack');

// Create a start state for the state machine
const startState = new stepfunctions.Pass(stack, 'StartState');

// Setup the Lambda function props
const lambdaFunctionProps = {
  runtime: lambda.Runtime.NODEJS_10_X,
  handler: 'index.handler',
  code: lambda.Code.fromAsset(`${__dirname}/lambda`)
};

// Setup the Lambda function
const fn = defaults.deployLambdaFunction(stack, lambdaFunctionProps);

// Create a new log group to use
const lg = new LogGroup(stack, 'StMacLogGroup');

// Setup the pattern props
const props: LambdaToStepFunctionProps = {
    existingLambdaObj: fn,
    stateMachineProps: {
      stateMachineType: stepfunctions.StateMachineType.EXPRESS,
      definition: startState,
      logs: {
        destination: lg,
        level: stepfunctions.LogLevel.FATAL,
        includeExecutionData: false
      }
    }
};

// Add the pattern
new LambdaToStepFunction(stack, 'test-lambda-step-function-stack', props);

// Synth the app
app.synth();

Let me know if I'm missing anything with the config here! If so, could you please share more details around steps to reproduce and a larger code sample? Thanks!

@knihit
Copy link
Member Author

knihit commented Dec 21, 2020

@hayesry , in the lambda function you defined, can you include the timeout propery as well and specify a 'Duration'.

const lambdaFunctionProps = {
  runtime: lambda.Runtime.NODEJS_10_X,
  handler: 'index.handler',
  code: lambda.Code.fromAsset(`${__dirname}/lambda`),
  timeout: Duration.minutes(5)
};

@hayesry
Copy link
Contributor

hayesry commented Dec 21, 2020

@knihit - Added the timeout: Duration.minutes(5) property to the lambdaFunctionProps but the code still built and synthesized on version 1.75.0. I'm running this combination of commands:

npm run build && cdk synth --app integ.existing-function.js

Let me know if I'm missing anything there?

@hayesry
Copy link
Contributor

hayesry commented Dec 21, 2020

++ Current code is:

import { App, Stack, Duration } from "@aws-cdk/core";
import { LambdaToStepFunction, LambdaToStepFunctionProps } from "../lib";
import * as lambda from '@aws-cdk/aws-lambda';
import * as stepfunctions from '@aws-cdk/aws-stepfunctions';
import * as defaults from '@aws-solutions-constructs/core';

import { LogGroup } from '@aws-cdk/aws-logs';

// Setup the app and stack
const app = new App();
const stack = new Stack(app, 'test-lambda-step-function-stack');

// Create a start state for the state machine
const startState = new stepfunctions.Pass(stack, 'StartState');

// Setup the "existing" Lambda function props
const lambdaFunctionProps = {
  runtime: lambda.Runtime.NODEJS_10_X,
  handler: 'index.handler',
  code: lambda.Code.fromAsset(`${__dirname}/lambda`),
  timeout: Duration.minutes(5)
};

// Setup the "existing" Lambda function
const fn = defaults.deployLambdaFunction(stack, lambdaFunctionProps);

// Create a new log group to use
const lg = new LogGroup(stack, 'StMacLogGroup');

// Setup the pattern props
const props: LambdaToStepFunctionProps = {
    existingLambdaObj: fn,
    stateMachineProps: {
      stateMachineType: stepfunctions.StateMachineType.EXPRESS,
      definition: startState,
      logs: {
        destination: lg,
        level: stepfunctions.LogLevel.FATAL,
        includeExecutionData: false
      }
    }
};

// Add the pattern
new LambdaToStepFunction(stack, 'test-lambda-step-function-stack', props);

// Synth the app
app.synth();

@hnishar hnishar removed the in-progress This issue is being actively worked on label Dec 31, 2020
@hnishar
Copy link
Contributor

hnishar commented Dec 31, 2020

The fix for this bug is released in v1.79.0

@hnishar hnishar closed this as completed Dec 31, 2020
@dvd-88
Copy link

dvd-88 commented Feb 1, 2022

I think the bug is still present: any time the flagOverriddenDefaults function in source/patterns/@aws-solutions-constructs/core/lib/override-warning-service.ts is invoked with an object having a property of type Duration it throws this error.
I think the issue is that the aforementioned function calls const diff = deepdiff.diff(defaultProps, userProps, _prefilter); which in turn calls realTypeOf that, in case of an object of type Duration, it falls in this case: typeof subject.toString === 'function' && /^\/.*\//.test(subject.toString()). However Duration's toString method throws the error mentioned above:

toString() { return token_1.Token.asString(() => { throw new Error('Duration.toString() was used, but .toSeconds, .toMinutes or .toDays should have been called instead'); }, { displayHint:${this.amount} ${this.unit.label}}); }

In the end I think the issue is that deep-diff and Duration are not compatible in the current state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants