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

(stepfunctions-tasks): LambdaInvoke - explicit null is not passed to lambda when using #16253

Closed
TomasChmelik opened this issue Aug 27, 2021 · 19 comments · Fixed by #28661
Closed
Assignees
Labels
@aws-cdk/aws-stepfunctions-tasks bug This issue is a bug. effort/small Small work item – less than a day of effort p2

Comments

@TomasChmelik
Copy link

TomasChmelik commented Aug 27, 2021

When I try to pass null as some parameter of a lambda, it gets removed from resulting task's parameters.

It might have same result for javascript lambdas, but in Python there is a difference between key not being in input event and key being there with value null (or None in Python's case).

This issue can be worked around by adding additional checks in the lambda, but that would add additional step needed for migration to CDK.

Reproduction Steps

from aws_cdk import aws_lambda
from aws_cdk import aws_stepfunctions
from aws_cdk import aws_stepfunctions_tasks


some_lambda = aws_lambda.Function.from_function_arn(scope, 'SomeLambda', 'arn:aws:lambda:eu-central-1:123456789:function:some-lambda')

aws_stepfunctions_tasks.LambdaInvoke(
    scope,
    'Lambda',
    lambda_function=some_lambda,
    payload=aws_stepfunctions.TaskInput.from_object({
        'nullParameter': None,
        'notNullParameter': 'not null'
    }),
    payload_response_only=True,
    retry_on_service_exceptions=False
)

What did you expect to happen?

{
    "Type": "Task",
    "Resource": "arn:aws:lambda:eu-central-1:123456789:function:some-lambda",
    "Parameters": {
        "nullParameter": null,
        "notNullParameter": "not null"
    }
}

What actually happened?

{
    "Type": "Task",
    "Resource": "arn:aws:lambda:eu-central-1:123456789:function:some-lambda",
    "Parameters": {
        "notNullParameter": "not null"
    }
}

Environment

  • CDK CLI Version :1.118.0
  • Framework Version:???
  • Node.js Version:v15.14.0
  • OS :Ubuntu 18.04.5 LTS
  • Language (Version):Python 3.7.4

Other


This is 🐛 Bug Report

@TomasChmelik TomasChmelik added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Aug 27, 2021
@TomasChmelik TomasChmelik changed the title (stepfunctions-tasks): Explicit null is not passed to lambda when using LambdaInvoke (stepfunctions-tasks): LambdaInvoke - explicit null is not passed to lambda when using Aug 27, 2021
@BenChaimberg BenChaimberg removed the @aws-cdk/aws-lambda Related to AWS Lambda label Aug 27, 2021
@BenChaimberg BenChaimberg added guidance Question that needs advice or information. and removed bug This issue is a bug. labels Aug 27, 2021
@BenChaimberg
Copy link
Contributor

@peterwoodworth Can you help with alternatives to this pattern? I don't believe there is anything we can do about this due to TypeScript limitations

@BenChaimberg BenChaimberg removed the needs-triage This issue or PR still needs to be triaged. label Aug 27, 2021
@TomasChmelik
Copy link
Author

TomasChmelik commented Aug 30, 2021

Typescript has null and undefined which are not strictly equal, so you could use one of them to specify that property is not there (probably undefined, since when you try to access property on object which is not there, you will also get an undefined) and other one for null valued property.

But I can't find where the TaskInput is being serialized into output to check if this is possible.

@BenChaimberg BenChaimberg added bug This issue is a bug. effort/small Small work item – less than a day of effort p1 and removed guidance Question that needs advice or information. labels Aug 30, 2021
@BenChaimberg
Copy link
Contributor

Ah, my bad, glazed over the value in particular. I believe it is this condition within renderObject that needs to be fixed. Only undefined should be ignored, null should be returned.

} else if (value === null || value === undefined) {
// Nothing

@readybuilderone
Copy link
Contributor

Hi @BenChaimberg , I would like to try to solve this problem, I wonder if you mind?

@BenChaimberg
Copy link
Contributor

Please go ahead! We are always happy to take contributions.

@BenChaimberg BenChaimberg removed their assignment Sep 9, 2021
@kaizencc
Copy link
Contributor

@readybuilderone are you still planning on submitting a PR to fix this bug?

@jaresuth
Copy link

@kaizen3031593 - I am actively working on a fix for this. Can you assign this issue to me?

@kaizencc
Copy link
Contributor

@jaresuth are you still working on this?

@fab-mindflow
Copy link

Bump. Any news on this?
We are also facing the same issue, not able to distinguish null from undefined.

@krish00back
Copy link

Is this fixed? We also facing this issue.

@gakinson
Copy link

We are experiencing this as well.

@wancaibida
Copy link

I faced a similar error when creating a pass state with null values. A quick solution is using sfn.JsonPath.stringToJson("") instead of null

        const passStateStoreItemLastEvaluatedKey = new sfn.Pass(this, `${resourcePrefix}-pass-store-item`, {
            parameters: {
                "Count.$": "$.Count",
                "EmailResult.$": "$.EmailResult",
                "projectId.$": "$.projectId",
                "ItemResult": {
                    "LastEvaluatedKey": sfn.JsonPath.stringToJson("")
                },
                "walletAddress.$": "$.walletAddress",
                "networkType.$": "$.networkType"
            },
            comment: "Transform store email result"
        });

@mRamzii
Copy link

mRamzii commented Aug 8, 2023

I faced a similar error when creating a pass state with null values. A quick solution is using sfn.JsonPath.stringToJson("") instead of null

        const passStateStoreItemLastEvaluatedKey = new sfn.Pass(this, `${resourcePrefix}-pass-store-item`, {
            parameters: {
                "Count.$": "$.Count",
                "EmailResult.$": "$.EmailResult",
                "projectId.$": "$.projectId",
                "ItemResult": {
                    "LastEvaluatedKey": sfn.JsonPath.stringToJson("")
                },
                "walletAddress.$": "$.walletAddress",
                "networkType.$": "$.networkType"
            },
            comment: "Transform store email result"
        });

Yup having the same issue here, nice workaround !

@morrijm4
Copy link
Contributor

morrijm4 commented Sep 8, 2023

I faced a similar error when creating a pass state with null values. A quick solution is using sfn.JsonPath.stringToJson("") instead of null

        const passStateStoreItemLastEvaluatedKey = new sfn.Pass(this, `${resourcePrefix}-pass-store-item`, {
            parameters: {
                "Count.$": "$.Count",
                "EmailResult.$": "$.EmailResult",
                "projectId.$": "$.projectId",
                "ItemResult": {
                    "LastEvaluatedKey": sfn.JsonPath.stringToJson("")
                },
                "walletAddress.$": "$.walletAddress",
                "networkType.$": "$.networkType"
            },
            comment: "Transform store email result"
        });

Yup having the same issue here, nice workaround !

Same issue with me as well, thanks for the workaround!

@mrgrain
Copy link
Contributor

mrgrain commented Jan 12, 2024

Can y'all try sfn.JsonPath.DISCARD where you want to pass the null value that doesn't exist in Python?

@TomasChmelik
Copy link
Author

Interesting workaround. Not something I would expect sfn.JsonPath.DISCARD to do, but it works

@mrgrain
Copy link
Contributor

mrgrain commented Jan 12, 2024

Interesting workaround. Not something I would expect sfn.JsonPath.DISCARD to do, but it works

Agreed. It's named after this feature which at the time might have been the only (known) use case for null in ASL.

@paulhcsun is looking into DX improvements on the linked PR.

@mergify mergify bot closed this as completed in #28661 Jan 16, 2024
mergify bot pushed a commit that referenced this issue Jan 16, 2024
#28661)

Update docs for use of `sfn.JsonPath.DISCARD` in place of `null` in `TaskInput.fromObject`s field value for languages that do not support `null` like Python. 

Doc: https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_stepfunctions.TaskInput.html#:~:text=to%20a%20task.-,static%20fromObject(obj),-public%20static%20fromObject

Closes #16253.

----

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

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@anentropic
Copy link

Can y'all try sfn.JsonPath.DISCARD where you want to pass the null value that doesn't exist in Python?

null in Python is None

it is JS's undefined which doesn't exist in Python surely?

Nones in Python should be treated as explicit null values

whereas operations which return undefined in JS usually raise an exception in Python

and cases (e.g. default values for function args) where you need to distinguish "not supplied" from "explicit None supplied" would usually use a sentinel object for the former case

e.g.

NOT_SUPPLIED = object()

def myfunc(arg=NOT_SUPPLIED):
    if arg is None:
        ...
    elif arg is NOT_SUPPLIED:
        ...
    else:
        ...

using the bizarrely named sfn.JsonPath.DISCARD value (discard what? isn't it doing the opposite of discarding the explicit null?) where you want an explicit null in the input JSON seems completely back-to-front from conventional practice... even in JS wouldn't it be more normal to treat null as an explicit null?

contrary to the idea that "null value doesn't exist in Python" it feels like real source of the problem must stem from core code mistakenly treating undefined vs null as the same thing rather than distinct? why else would you need a special case for "explicitly null" values?

(sfn.JsonPath.stringToJson("") returning JSON null instead of an error is also a bit weird, but at least the more explicit sfn.JsonPath.stringToJson("null") works as expected and makes more sense than sfn.JsonPath.DISCARD when reading the code, so I will use that for now)

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