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

Error not properly thrown with bad stream definition for DynamoDB #8111

Closed
ktwbc opened this issue Aug 20, 2020 · 1 comment
Closed

Error not properly thrown with bad stream definition for DynamoDB #8111

ktwbc opened this issue Aug 20, 2020 · 1 comment

Comments

@ktwbc
Copy link

ktwbc commented Aug 20, 2020

If you have an error in serverless.yml defining a bad stream type, the Error message handler does not work because it itself creates a secondary error because of a missing array.

If your serverless.yml definition is:

myDynamoStream:
  description: DynamoDB Stream of changes to table
  handler: src/controller/streams.myDynamoStream
  stackTags:
    Project: star
  memorySize: 1024
  timeout: 900
  events:
    - stream:
        type: star
        arn:  arn:19123example

Here my "type" due to a copy and paste error is not dynamodb or kinesis which are the only accepted types. However the error on compile is:

TypeError: Cannot read property 'Policies' of undefined
at /Volumes/dev/myprojectnode_modules/serverless/lib/plugins/aws/package/compile/events/stream/index.js:258:62
at Array.forEach ()
at /Volumes/dev/myproject/node_modules/serverless/lib/plugins/aws/package/compile/events/stream/index.js:99:28
at Array.forEach ()
at AwsCompileStreamEvents.compileStreamEvents

This code:

 if (streamType === 'dynamodb') {
              dynamodbStreamStatement.Resource.push(EventSourceArn);
            } else if (streamType === 'kinesis') {
              if (event.stream.consumer) {
                kinesisStreamWithConsumerStatement.Resource.push(EventSourceArn);
              } else {
                kinesisStreamStatement.Resource.push(EventSourceArn);
              }
            } else {
              const errorMessage = [
                `Stream event of function '${functionName}' had unsupported stream type of`,
                ` '${streamType}'. Valid stream event source types include 'dynamodb' and`,
                " 'kinesis'. Please check the docs for more info.",
              ].join('');
              throw new this.serverless.classes.Properties.Policies[0].PolicyDocument.Error(
                errorMessage
              );
            }

Will fail because this.serverless.classes.Properties.Policies is undefined. This should be replaced with just a simple

 throw new Error(errorMessage); 

which will work and you get the proper error:

 Error: Stream event of function 'myDynamoStream' had unsupported stream type of 'star'. Valid stream event source types include 'dynamodb' and 'kinesis'. Please check the docs for more info.
      at /Volumes/dev/myproject/node_modules/serverless/lib/plugins/aws/package/compile/events/stream/index.js:257:21
      at Array.forEach (<anonymous>)
      at /Volumes/dev/myproject/node_modules/serverless/lib/plugins/aws/package/compile/events/stream/index.js:99:28
      at Array.forEach (<anonymous>)
   

I did a PR for this but the application process I don't have time to go through right now and write tests, so I'm just filing it.

serverless.yml
# ⚠️⚠️ REPLACE THIS COMMENT WITH FULL serverless.yml CONTENT
⚠️⚠️ REPLACE WITH FULL COMMAND NAME output
⚠️⚠️ REPLACE WITH FULL COMMAND OUTPUT

Installed version

⚠️⚠️ REPLACE WITH `serverless --version` OUTPUT
@medikoo
Copy link
Contributor

medikoo commented Aug 21, 2020

@ktwbc great thanks for report.

Currently we're tackling such errors with config validation backed by JSON schema (and setting "configValidationMode: error will make process crash on any error, that'll become default at some point in a Framework).

Unfortunately we still don't have a schema for stream event type, see dedicated issue: #8034 (PR that implements it is welcome!)

I'm going to close it, as technically this will be solved with #8034

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants