More detailed transformation error output

[fatbasstard]

If the transformations generate pretty basic errors, for example the following "error":

  MyLambdaFunction:
    Type: "AWS::Serverless::Function"
    Properties:
      CodeUri: build
      FunctionName: "my-function-name"
      Handler: "my_module.handler"
      Runtime: "python3.6"
      Timeout: "30"
      Role: !ImportValue "mu-lambda-role"

Generates the following Transformation error:

Resource with id [MyLambdaFunction] is invalid. Type of property 'Timeout' is invalid.

It would be great if there errors can be extended with additional information so we can trace the error back to the actual resource/property (or perhaps even the line/column number) of the original SAM template that gave the error.

The transformations and a (more) detailed error output is really valuable for consomers like SAM-CLI (to perform the "validate" function), but also by the (new) CloudFormation Linter (cfn-python-lint).

This would allow consumers of SAM to give better and more direct feedback on errors in the SAM/CloudFormation template and make the usage of SAM easier.

Extending the InvalidDocumentException (I think this is the place where it could/should be added) with this information would be really appreciated:

https://github.com/awslabs/serverless-application-model/blob/master/samtranslator/model/exceptions.py

[brettstack]

What other details do you want in the error message? I don't think it will be possible to include the line+column numbers of the template in the error

[fatbasstard]

That would be ultime. But the original resource name, as a separate attribute in the exception would be a great start...

[brettstack]

Could you clarify? It looks like the original resource name MyLambdaFunction is in the error message. Maybe this isn't true for all exceptions?

[fatbasstard]

Good one, I'll take a look at the code. I'll get back on this ASAP 👍

[jfuss]

Hmm.. I does look like the Logical Id is in some of the exceptions, which I didn't realize. The InvalidEventException is the only one that does not have the 'main' logical id, it just have the id of the event. I see it being useful to add there, that is the logical_id of the Lambda Function, to add further clarity.

I think the exceptions can be modeled better though. Right now the error that is returned when you transform is InvalidDocumentException, which contains a list of lots of different Exceptions types. It might be better to split these lists into the different types to make consuming them easier.

I don't think it will be possible to include the line+column numbers of the template in the error

While I understand this may be hard for the service to do this, but locally this is possible. This can provide richer details and help customers quickly find the resources or conflicts that are in the template. I think it is worth while to at least explore what this is and how we could possible support it, even if we don't end up adding it for whatever reason.

[fatbasstard]

Got some more details:

---
AWSTemplateFormatVersion: '2010-09-09'
Transform: AWS::Serverless-2016-10-31
Resources:
  myBucket:
    Type: AWS::S3::Bucket
    Properties: {}
  myFunction:
    Type: AWS::Serverless::Function
    Properties:
      Handler: index.handler
      Runtime: nodejs4.3
      CodeUri: 's3://testBucket/mySourceCode.zip'
      AutoPublishAlias: live
      DeploymentPreference:
        Type: Linear10PercentEvery10Minutes
      Events:
        MyTimer:
          Type: Schedule
          Properties:
            Input: rate(5 minutes)

Returns:

Resource with id [myFunctionMyTimer] is invalid. Missing required property 'Schedule'.

There is a logical_id present indeed, but this is not always the "original resource", in in this case it's the Event (like @jfuss also just posted basically).

For cfn-lint the resource name could sufficient, we can handle that locally.

A bit more contextual enrichment in the Exceptions would make it (even) more useful for customers.

[jfuss]

I think more detailed errors can come out of our work on #1133. The team is looking at ways to do this.

Closing this in favor of #1133