Skip to content

Conversation

@aparajit-pratap
Copy link
Contributor

@aparajit-pratap aparajit-pratap commented Nov 21, 2018

Purpose

JIRA: https://jira.autodesk.com/browse/QNTM-4425
GH Issue: #8911

The crash occurs at the time of compilation of the custom node definition. It tries to build a return AST statement with the output from the code block node but since the code block output is null owing to the syntax error, an ArgumentNullException is thrown from the AstFactory.BuildReturnStatement, which apparently isn't caught anywhere and results in a crash. The fix is to build a NullNode if the rhs returned from the code block AST is null and then pass this NullNode as input to the BuildReturnStatement method to generate AST for a return = null; statement.

Declarations

Check these if you believe they are true

  • The code base is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning, and are documented in the API Changes document.

Reviewers

@mjkkirschner @QilongTang @ColinDayOrg

Adding a test case would be tricky here - it would be best to add a recordable test but I'm not sure if we can add these for a custom node workspace. Will check.

@mjkkirschner
Copy link
Member

@aparajit-pratap thanks for the explanation LGTM -
custom node recordable tests should be possible, there are commands for creating custom node workspaces and switching to them.

@QilongTang
Copy link
Contributor

LGTM

@aparajit-pratap aparajit-pratap merged commit 570486d into DynamoDS:master Nov 26, 2018
@aparajit-pratap aparajit-pratap deleted the customCrash branch November 26, 2018 17:28
@horatiubota horatiubota added the error/warning/crash Issues mentioning a Dynamo error, warning or crash message label Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

error/warning/crash Issues mentioning a Dynamo error, warning or crash message

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants