Skip to content

Commit

Permalink
fix(python): returning Promise<void> breaks runtime (#3916)
Browse files Browse the repository at this point in the history
The `EndResponse` struct did not supply a default value for the `result` property, making it required (despite it is typed `Any`, which `None` is valid for). Fix the schema to represent as `Optional[Any] = None` and add a compliance test to validate this scenario.

Note for readers here - async support is very much experimental and only partially imported in most jsii front-end languages. Caution is still advised if you are going to expose async APIs.



---

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
  • Loading branch information
RomainMuller committed Jan 19, 2023
1 parent e4c51c4 commit 209d10e
Show file tree
Hide file tree
Showing 13 changed files with 336 additions and 58 deletions.
3 changes: 2 additions & 1 deletion gh-pages/content/specification/6-compliance-report.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
This section details the current state of each language binding with respect to our standard compliance suite.


| number | test | java (98.36%) | golang (79.51%) | Dotnet | Python |
| number | test | java (97.56%) | golang (78.86%) | Dotnet | Python |
| ------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------- | -------------------------------------------- | ------ | ------ |
| 1 | asyncOverrides_overrideCallsSuper | 🟢 | [🔴](https://github.com/aws/jsii/issues/2670) |||
| 2 | [arrayReturnedByMethodCanBeRead]("Array created in the kernel can be queried for its elements") | 🟢 | 🟢 |||
Expand Down Expand Up @@ -129,3 +129,4 @@ This section details the current state of each language binding with respect to
| 120 | [downcasting]("Ensures unsafe-cast features work as expected") || 🟢 |||
| 121 | [strippedDeprecatedMemberCanBeReceived]("Ensures --strip-deprecated does not cause odd runtime errors") | 🟢 | 🟢 |||
| 122 | [exceptionMessage]("Verifies that custom exception names are correctly forwarded") | 🟢 | 🟢 |||
| 123 | [voidReturningAsync]("Verifies that returning Promise<void> is correctly handled") |||||
2 changes: 1 addition & 1 deletion packages/@jsii/python-runtime/src/jsii/_kernel/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ class EndRequest:
@attr.s(auto_attribs=True, frozen=True, slots=True)
class EndResponse:

result: Any
result: Optional[Any] = None


@attr.s(auto_attribs=True, frozen=True, slots=True)
Expand Down
9 changes: 9 additions & 0 deletions packages/@jsii/python-runtime/tests/test_compliance.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
StructParameterType,
AnonymousImplementationProvider,
UpcasingReflectable,
PromiseNothing,
)
from jsii_calc.cdk16625 import Cdk16625
from jsii_calc.cdk22369 import AcceptsPath
Expand Down Expand Up @@ -1354,3 +1355,11 @@ def test_stripped_deprecated_member_can_be_received():
def test_exception_message():
with pytest.raises(RuntimeError, match="Cannot find asset"):
AcceptsPath(source_path="A Bad Path")


def test_void_returning_async():
"""Verifies it's okay to return a Promise<void>."""

assert PromiseNothing().instance_promise_it() is None
## TODO: This is currently broken as code-gen is incorrect for static async.
# assert PromiseNothing.promise_it() is None
10 changes: 10 additions & 0 deletions packages/jsii-calc/lib/compliance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3131,3 +3131,13 @@ export interface ParamShadowsBuiltinsProps {
readonly booleanProperty: boolean;
readonly structProperty: StructA;
}

export class PromiseNothing {
public static async promiseIt(): Promise<void> {
return Promise.resolve();
}

public async instancePromiseIt(): Promise<void> {
return PromiseNothing.promiseIt();
}
}
46 changes: 45 additions & 1 deletion packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -11823,6 +11823,50 @@
],
"symbolId": "lib/calculator:Power"
},
"jsii-calc.PromiseNothing": {
"assembly": "jsii-calc",
"docs": {
"stability": "stable"
},
"fqn": "jsii-calc.PromiseNothing",
"initializer": {
"docs": {
"stability": "stable"
}
},
"kind": "class",
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 3135
},
"methods": [
{
"async": true,
"docs": {
"stability": "stable"
},
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 3136
},
"name": "promiseIt",
"static": true
},
{
"async": true,
"docs": {
"stability": "stable"
},
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 3140
},
"name": "instancePromiseIt"
}
],
"name": "PromiseNothing",
"symbolId": "lib/compliance:PromiseNothing"
},
"jsii-calc.PropertyNamedProperty": {
"assembly": "jsii-calc",
"docs": {
Expand Down Expand Up @@ -18799,5 +18843,5 @@
}
},
"version": "3.20.120",
"fingerprint": "kmlc5is+t/xffykk7b4m8jurrxFDkj9pjv2cW/V1J50="
"fingerprint": "EH7xszNdCh9PCFUZ8Foi7g2CPhdrKeZm8CQaUCNv4GQ="
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 209d10e

Please sign in to comment.