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

rosetta extract command doesn't respect strict metadata entry #2861

Closed
5 tasks
BenChaimberg opened this issue May 28, 2021 · 1 comment · Fixed by #2863
Closed
5 tasks

rosetta extract command doesn't respect strict metadata entry #2861

BenChaimberg opened this issue May 28, 2021 · 1 comment · Fixed by #2863
Labels
bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. p2

Comments

@BenChaimberg
Copy link
Contributor

🐛 Bug Report

Affected Languages

  • TypeScript or Javascript
  • Python
  • Java
  • .NET (C#, F#, ...)
  • Go

General Information

  • JSII Version: 1.29.0
  • Platform: macOS 10.15.7

What is the problem?

The README for jsii-rosetta states:

Package owners can enable enforcement of compiling code sample by setting the jsii.rosetta.strict assembly metadata entry to true

However, this does not seem to be the case. For example, adding the corresponding entry to the @aws-cdk/aws-lambda module and running jsii-rosetta extract produces no error messages and an exit code of 0, while running the same command with the --strict flag produces a whole bunch of diagnostics (77 at the time of writing) and produces an exit code of 1. This can easily be reproduced with a jsii projen project by adding the following to the .projenrc.js:

Object.assign(project.manifest['jsii'], {
  metadata: {
    jsii: {
      rosetta: {
        strict: true
      }
    }
  }
});

Then, any non-compiling snippet in the README or otherwise will not fail when running jsii-rosetta extract.

There seem to be two separate issues that are causing this problem, one that is preventing compilation and one that is preventing failure when compilation fails.

  1. During the fixturize command that is used to combine snippets with fixtures, when returning the fixturized snippet, does not copy the strict flag that marks whether the snippet should be compiled and cause a failure. This can be fixed by instead returning
{                                                                                                                                                                      
  ...snippet,                                                                                                                                                                      
  completeSource: source,                                                                                                                                                          
  parameters,                                                                                                                                                                      
};
  1. During translation, the worker_threads module is used to parallelize translation of snippets. Each worker passes the compiler "diagnostics" (which contain any failures that may have arose during compilation of a snippet) in its return value. These diagnostics should be annotated by the translator to contain a special Symbol property that marks the diagnostic as "strict" if the extraction run was configured as such. Unfortunately, the message passing protocol that worker_threads uses does not support object properties that are Symbols. This means that the diagnostics that are returned do not satisfy the criteria for being an error and so extraction does not fail. This can potentially be fixed by defining a diagnostic "schema" in the same vein as TranslatedSnippetSchema which seems to exist for serialization purposes only.

Verbose Log

N/A – should be getting an error, but passing silently.

@BenChaimberg BenChaimberg added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels May 28, 2021
@RomainMuller RomainMuller removed their assignment Jun 24, 2021
@mergify mergify bot closed this as completed in #2863 Jun 29, 2021
mergify bot pushed a commit that referenced this issue Jun 29, 2021
The `jsii.rosetta.strict` assembly metadata entry was not being respected by the `extract` command due to two bugs:
1. Fixturizing (the process by which snippets are enriched by a separate fixture) did not transfer the "strict" flag from the original snippet to the fixturized snippet. Fix: transfer the strict flag to the fixturized snippet.
2. Translation of snippets to other languages using the `worker_threads` node module involves sending compiler diagnostics across the wire. These diagnostics should be annotated with a "strict brand" that marks them as failing compilation and allows further steps to fail if such a diagnostic exist. Because the brand was a [Symbol](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol), it is not correctly serialized. Fix: make the brand a string so it can be properly serialized.

fixes #2861

---

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
@github-actions
Copy link
Contributor

⚠️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. p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants