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

(aws_stepfunctions): Overriding to_stete_json() fails if result_path=sfn.JsonPath.DISCARD #14639

Closed
wisaac407 opened this issue May 11, 2021 · 4 comments · Fixed by #24593
Closed
Labels
@aws-cdk/aws-stepfunctions Related to AWS StepFunctions bug This issue is a bug. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. p2

Comments

@wisaac407
Copy link

Overriding to_stete_json() on the DynamoUpdateItem construct raises an exception if result_path=sfn.JsonPath.DISCARD

Reproduction Steps

Run cdk synth on the provided sample

from aws_cdk import aws_dynamodb as dynamodb
from aws_cdk import aws_stepfunctions as sfn
from aws_cdk import aws_stepfunctions_tasks as tasks
from aws_cdk import core


class CustomDynamoUpdateItem(tasks.DynamoUpdateItem):
    def to_state_json(self):
        return super().to_state_json()


class TestStack(core.Stack):
    def __init__(self, scope: core.Construct, construct_id: str, **kwargs) -> None:
        super().__init__(scope, construct_id, **kwargs)

        table = dynamodb.Table(
            self,
            "Table",
            partition_key=dynamodb.Attribute(
                name="id", type=dynamodb.AttributeType.STRING
            ),
        )
        sfn.StateMachine(
            self,
            "StateMachine",
            definition=CustomDynamoUpdateItem(
                self,
                "Update Item",
                table=table,
                result_path=sfn.JsonPath.DISCARD,
                key={
                    "id": tasks.DynamoAttributeValue.from_string(
                        sfn.JsonPath.string_at("$.id")
                    )
                },
                update_expression="SET value=:value",
                expression_attribute_values={
                    ":value": tasks.DynamoAttributeValue.from_string(
                        sfn.JsonPath.string_at("$.value")
                    )
                },
            ),
        )

What did you expect to happen?

It should produce a valid cloudformation stack.

What actually happened?

It produces this error:

jsii.errors.JavaScriptError: 
  Error: Got 'undefined' for non-optional instance of {"type":{"primitive":"json"}}
      at nullAndOk (/private/var/folders/m1/lyhg812d7wn8tgcxk8_xmhbm0000gp/T/tmp84la7qu9/lib/program.js:9318:23)
      at Object.deserialize (/private/var/folders/m1/lyhg812d7wn8tgcxk8_xmhbm0000gp/T/tmp84la7qu9/lib/program.js:8880:25)
      at Kernel._toSandbox (/private/var/folders/m1/lyhg812d7wn8tgcxk8_xmhbm0000gp/T/tmp84la7qu9/lib/program.js:8505:69)
      at /private/var/folders/m1/lyhg812d7wn8tgcxk8_xmhbm0000gp/T/tmp84la7qu9/lib/program.js:9007:81
      at mapValues (/private/var/folders/m1/lyhg812d7wn8tgcxk8_xmhbm0000gp/T/tmp84la7qu9/lib/program.js:9341:35)
      at Object.deserialize (/private/var/folders/m1/lyhg812d7wn8tgcxk8_xmhbm0000gp/T/tmp84la7qu9/lib/program.js:9007:36)
      at Object.deserialize (/private/var/folders/m1/lyhg812d7wn8tgcxk8_xmhbm0000gp/T/tmp84la7qu9/lib/program.js:8884:59)
      at Kernel._toSandbox (/private/var/folders/m1/lyhg812d7wn8tgcxk8_xmhbm0000gp/T/tmp84la7qu9/lib/program.js:8505:69)
      at DynamoUpdateItem.value (/private/var/folders/m1/lyhg812d7wn8tgcxk8_xmhbm0000gp/T/tmp84la7qu9/lib/program.js:8333:41)
      at StateGraph.toGraphJson (/private/var/folders/m1/lyhg812d7wn8tgcxk8_xmhbm0000gp/T/jsii-kernel-6nqdST/node_modules/@aws-cdk/aws-stepfunctions/lib/state-graph.js:106:43)

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "app.py", line 15, in <module>
    TestStack(app, "TestStack")
  File "/Users/weaveri/test/.venv/lib/python3.8/site-packages/jsii/_runtime.py", line 83, in __call__
    inst = super().__call__(*args, **kwargs)
  File "/Users/weaveri/test/test/test_stack.py", line 23, in __init__
    sfn.StateMachine(
  File "/Users/weaveri/test/.venv/lib/python3.8/site-packages/jsii/_runtime.py", line 83, in __call__
    inst = super().__call__(*args, **kwargs)
  File "/Users/weaveri/test/.venv/lib/python3.8/site-packages/aws_cdk/aws_stepfunctions/__init__.py", line 5100, in __init__
    jsii.create(StateMachine, self, [scope, id, props])
  File "/Users/weaveri/test/.venv/lib/python3.8/site-packages/jsii/_kernel/__init__.py", line 287, in create
    obj.__jsii_ref__ = _callback_till_result(self, response, CreateResponse)
  File "/Users/weaveri/test/.venv/lib/python3.8/site-packages/jsii/_kernel/__init__.py", line 220, in _callback_till_result
    response = kernel.sync_complete(response.cbid, None, result, response_type)
  File "/Users/weaveri/test/.venv/lib/python3.8/site-packages/jsii/_kernel/__init__.py", line 386, in sync_complete
    return self.provider.sync_complete(
  File "/Users/weaveri/test/.venv/lib/python3.8/site-packages/jsii/_kernel/providers/process.py", line 382, in sync_complete
    resp = self._process.send(_CompleteRequest(complete=request), response_type)
  File "/Users/weaveri/test/.venv/lib/python3.8/site-packages/jsii/_kernel/providers/process.py", line 326, in send
    raise JSIIError(resp.error) from JavaScriptError(resp.stack)
jsii.errors.JSIIError: Got 'undefined' for non-optional instance of {"type":{"primitive":"json"}}
Subprocess exited with error 1

Environment

  • CDK CLI Version : 1.103.0
  • Framework Version: 1.103.0
  • Node.js Version: v14.16.0
  • OS : macOS Catalina
  • Language (Version): Python 3.8.2

Other

I'm attempting to use States.Format() in the update expression to parameterize the index of a dynamodb list based on the index of a Map state. Because States.Format() isn't supported yet (#11286) my attempted work-around is to override to_stete_json(). However, that's producing the above error.


This is 🐛 Bug Report

@wisaac407 wisaac407 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels May 11, 2021
@wisaac407 wisaac407 changed the title (aws_stepfunctions): short issue description (aws_stepfunctions): Overriding to_stete_json() on the DynamoUpdateItem construct raises an exception if result_path=sfn.JsonPath.DISCARD May 11, 2021
@wisaac407 wisaac407 changed the title (aws_stepfunctions): Overriding to_stete_json() on the DynamoUpdateItem construct raises an exception if result_path=sfn.JsonPath.DISCARD (aws_stepfunctions): Overriding to_stete_json() fails if result_path=sfn.JsonPath.DISCARD May 11, 2021
@peterwoodworth peterwoodworth added the @aws-cdk/aws-stepfunctions Related to AWS StepFunctions label May 11, 2021
@shivlaks shivlaks added p2 and removed needs-triage This issue or PR still needs to be triaged. labels May 12, 2021
@robjongbloed
Copy link

I am having the same problem overriding to_state_json in the LambdaInvoke task.

Error: Got \'undefined\' for non-optional instance of {"type":{"primitive":"json"}}
    at nullAndOk (/private/var/folders/wt/7pp002_x6l950ysl45b1k7ljl8ytr0/T/tmp_c0tz4nx/lib/program.js:9531:23)
    at Object.deserialize (/private/var/folders/wt/7pp002_x6l950ysl45b1k7ljl8ytr0/T/tmp_c0tz4nx/lib/program.js:9093:25)
    at Kernel._toSandbox (/private/var/folders/wt/7pp002_x6l950ysl45b1k7ljl8ytr0/T/tmp_c0tz4nx/lib/program.js:8718:69)
    at /private/var/folders/wt/7pp002_x6l950ysl45b1k7ljl8ytr0/T/tmp_c0tz4nx/lib/program.js:9220:81
    at mapValues (/private/var/folders/wt/7pp002_x6l950ysl45b1k7ljl8ytr0/T/tmp_c0tz4nx/lib/program.js:9554:35)
    at Object.deserialize (/private/var/folders/wt/7pp002_x6l950ysl45b1k7ljl8ytr0/T/tmp_c0tz4nx/lib/program.js:9220:36)
    at Object.deserialize (/private/var/folders/wt/7pp002_x6l950ysl45b1k7ljl8ytr0/T/tmp_c0tz4nx/lib/program.js:9097:59)
    at Kernel._toSandbox (/private/var/folders/wt/7pp002_x6l950ysl45b1k7ljl8ytr0/T/tmp_c0tz4nx/lib/program.js:8718:69)
    at mapJsonValue (/private/var/folders/wt/7pp002_x6l950ysl45b1k7ljl8ytr0/T/tmp_c0tz4nx/lib/program.js:9120:37)
    at Array.map (<anonymous>)

If interested, this is to add the unsupported "TimeoutSecondsPath" field.

@robjongbloed
Copy link

robjongbloed commented Sep 7, 2021

To assist anyone else with this issue, I have worked out what the cause is.

When you use JsonPath.DISCARD as the value for result_path, then that gets translated in the to_state_json() function as "ResultPath": None which then blows up the deserialiser in the TypeScript side. This bug should be fixed.

As a workaround, leaving the result_path out completely, or explicitly set it to None, seems to fix the issue, as then the field in to_state_json() is left out. Note, I think this means the result ends up in '$' which might blow something else up in your use case. You can also explicitly set it to something like "$.discardedResult".

@github-actions
Copy link

github-actions bot commented Sep 7, 2022

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Sep 7, 2022
@beck3905
Copy link

beck3905 commented Mar 8, 2023

I am running into this same issue with extending the to_state_json function of the Map state. I found the Typescript code that removes any fields with undefined values during the stack.resolve() method. It seems that Python None gets translated to Typescript undefined rather than Typescript null. I'm not sure how to fix this issue since if I don't discard my Map state output it will be too large and fail my state machine execution.

RomainMuller added a commit that referenced this issue Mar 13, 2023
If any part of a state's JSON representation is `null`, that value will
be replaced by `undefined` when jsii sends data to the other language,
resulting in a change of semantics.

Multi-language APIs cannot differentiate between `null` and `undefined`
as non-JS languages typically fail to distinguish between them... In
order to address that, a `JsonNull` value was added which serializes to
`null` (via Javascript's standard `toJSON` method), which must be used
in such cases where `null` may need to cross the language boundary.

The `JsonPath.DISCARD` value is now a string-token representation of the
`JsonNull` instance.

Fixes #14639
RomainMuller added a commit that referenced this issue Mar 13, 2023
If any part of a state's JSON representation is `null`, that value will
be replaced by `undefined` when jsii sends data to the other language,
resulting in a change of semantics.

Multi-language APIs cannot differentiate between `null` and `undefined`
as non-JS languages typically fail to distinguish between them... In
order to address that, a `JsonNull` value was added which serializes to
`null` (via Javascript's standard `toJSON` method), which must be used
in such cases where `null` may need to cross the language boundary.

The `JsonPath.DISCARD` value is now a string-token representation of the
`JsonNull` instance.

Fixes #14639
mergify bot pushed a commit that referenced this issue Mar 13, 2023
If any part of a state's JSON representation is `null`, that value will be replaced by `undefined` when jsii sends data to the other language, resulting in a change of semantics.

Multi-language APIs cannot differentiate between `null` and `undefined` as non-JS languages typically fail to distinguish between them... In order to address that, a `JsonNull` value was added which serializes to `null` (via Javascript's standard `toJSON` method), which must be used in such cases where `null` may need to cross the language boundary.

The `JsonPath.DISCARD` value is now a string-token representation of the `JsonNull` instance.

Fixes #14639
Fixes aws/jsii#3999

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
mergify bot pushed a commit to aws/jsii that referenced this issue Mar 13, 2023
The JSON serialization class is a quasi-synonym for `Map<string, any>`, except it is meant to accept any valid JSON object, including one with `null` values. The serializer was however interpreting these as `Map<string, JSON>` when wired as a `$jsii.map` envelope, and this serialization class does NOT allow for null values.

Addresses the "easy" part of aws/aws-cdk#14639 (more complicated is that it would need to preserve `null` across the process boundary, which is currently not possible).

---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
homakk pushed a commit to homakk/aws-cdk that referenced this issue Mar 28, 2023
If any part of a state's JSON representation is `null`, that value will be replaced by `undefined` when jsii sends data to the other language, resulting in a change of semantics.

Multi-language APIs cannot differentiate between `null` and `undefined` as non-JS languages typically fail to distinguish between them... In order to address that, a `JsonNull` value was added which serializes to `null` (via Javascript's standard `toJSON` method), which must be used in such cases where `null` may need to cross the language boundary.

The `JsonPath.DISCARD` value is now a string-token representation of the `JsonNull` instance.

Fixes aws#14639
Fixes aws/jsii#3999

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-stepfunctions Related to AWS StepFunctions bug This issue is a bug. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants