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

jsii 1.86.0 incorrectly infers the type of an object #4203

Closed
psychon opened this issue Aug 2, 2023 · 2 comments · Fixed by #4204
Closed

jsii 1.86.0 incorrectly infers the type of an object #4203

psychon opened this issue Aug 2, 2023 · 2 comments · Fixed by #4204
Labels
bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.

Comments

@psychon
Copy link

psychon commented Aug 2, 2023

Describe the bug

I have a larg-ish Python CDK stack which uses aws_cdk.pipelines.CodePipelineSource.s3() and then calls .source_attribute() on the result.

Expected Behavior

cdk synth continues working.

Current Behavior

cdk synth fails with AttributeError: 'Step' object has no attribute 'source_attribute'.

Reproduction Steps

Sorry, I tried to produce a self-contained example. However, this app contains another, large-ish stack. Removing this large stack fixes the problem. At this point, I do not know how to debug this some more.

I also tried comparing the output when running with JSII_DEBUG=1 with the working and non-working case, but that didn't provide any insights either.

Edit: See my first comment below for an actual reproducer.

Possible Solution

Suggest a fix/reason for the bug

No idea about a fix, but "reason why I think this is a bug in jsii": pip install jsii==1.85.0 makes things work while 1.86.0 breaks again. No other commands run between the two versions.

Additional Information/Context

  • With jsii 1.85.0 everything works fine.
  • With jsii 1.86.0 my code started failing with AttributeError: 'Step' object has no attribute 'source_attribute'.
  • When print()ing the return value of aws_cdk.pipelines.CodePipelineSource.s3()
    • With jsii 1.85.0 this shows a type of aws_cdk.pipelines._CodePipelineSourceProxy
    • With jsii 1.86.0 this shows indeed a type of aws_cdk.pipelines.Step which indeed does not have source_attribute.

I don't know what magic jsii is doing to infer types, but somehow this magic now stops one step too early.

Also: #4202 sounds loosely related since it is also about a wrong class and also talks about 1.85.0 working and 1.86.0 breaking. If the reproducing instructions there are correct, that would definitively be the simpler reproducer.

If you have some suggestions on how to debug this some more, I can do so. But I cannot share the whole CDK app.

SDK version used

1.86.0

Environment details (OS name and version, etc.)

Ubuntu 23.04

@psychon psychon added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Aug 2, 2023
@psychon
Copy link
Author

psychon commented Aug 2, 2023

Ah, I found a reproducer! Create a new CDK app with cdk init --language python and apply the following changes:

diff --git a/app.py b/app.py
index 6c9b641..efc30af 100644
--- a/app.py
+++ b/app.py
@@ -2,11 +2,37 @@
 import os
 
 import aws_cdk as cdk
+from aws_cdk import (
+    # Duration,
+    Stack,
+    # aws_sqs as sqs,
+    aws_s3,
+    pipelines
+)
 
 from test_app.test_app_stack import TestAppStack
+from aws_cdk import aws_codepipeline as codepipeline
+from constructs import Construct
+import jsii
+
+
+@jsii.implements(pipelines.ICodePipelineActionFactory)
+class FooStep(pipelines.Step):
+    def __init__(self):
+        super().__init__("foobarstep")
+
+    def produce_action(self, stage: codepipeline.IStage, options: pipelines.ProduceActionOptions):
+        return pipelines.CodePipelineActionFactoryResult(run_orders_consumed=1)
+
+
+class FooStack(cdk.Stack):
+    def __init__(self, scope: Construct, construct_id: str, **kwargs) -> None:
+        super().__init__(scope, construct_id, **kwargs)
+        FooStep()
 
 
 app = cdk.App()
+FooStack(app, "FooStack")
 TestAppStack(app, "TestAppStack",
     # If you don't specify 'env', this stack will be environment-agnostic.
     # Account/Region-dependent features and context lookups will not work,
diff --git a/test_app/test_app_stack.py b/test_app/test_app_stack.py
index 7e231e7..66e8885 100644
--- a/test_app/test_app_stack.py
+++ b/test_app/test_app_stack.py
@@ -2,6 +2,8 @@ from aws_cdk import (
     # Duration,
     Stack,
     # aws_sqs as sqs,
+    aws_s3,
+    pipelines
 )
 from constructs import Construct
 
@@ -17,3 +19,9 @@ class TestAppStack(Stack):
         #     self, "TestAppQueue",
         #     visibility_timeout=Duration.seconds(300),
         # )
+
+
+        bucket = aws_s3.Bucket(scope=self, id="bucket", bucket_name="bucket", versioned=True)
+        source = pipelines.CodePipelineSource.s3(bucket=bucket, object_key="foobar")
+        print(source)
+        print(source.source_attribute("VersionId"))

Output of cdk ls with jsii 1.85:

<aws_cdk.pipelines._CodePipelineSourceProxy object at 0x7fa87e9a1550>
${Token[TOKEN.664]}
FooStack
TestAppStack

Output of cdk ls with jsii 1.86:

<aws_cdk.pipelines.Step object at 0x7ff6e27ad750>                           
Traceback (most recent call last):
  File "/tmp/test-app/app.py", line 36, in <module>
    TestAppStack(app, "TestAppStack",
  File "/tmp/foo-test/cdk-venv/lib/python3.11/site-packages/jsii/_runtime.py", line 118, in __call__
    inst = super(JSIIMeta, cast(JSIIMeta, cls)).__call__(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/test-app/test_app/test_app_stack.py", line 27, in __init__
    print(source.source_attribute("VersionId"))
          ^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'Step' object has no attribute 'source_attribute'

RomainMuller added a commit that referenced this issue Aug 2, 2023
The registration was done on the object's prototype, and the value from the constructor was always used, even if that was inherited, such that if a parent type had already been resolved previously, all child types would use the parent's FQN instead of their own.

Addressed this by instead storing the associations in an external WeakMap, and added a test case to validate correct behavior.

Fixes aws/aws-cdk#26604
Fixes #4202
Fixes #4203
@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2023

⚠️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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant