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

add try catch to CLR Object Marshaler getStringValue #10952

Merged
merged 2 commits into from
Jul 30, 2020

Conversation

mjkkirschner
Copy link
Member

@mjkkirschner mjkkirschner commented Jul 29, 2020

Purpose

Address the situation where a ToString call on a clr object that is not null results in an exception - it turns out this happens quite frequently in Autocad and Revit.

Declarations

Check these if you believe they are true

  • The codebase 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.
  • This PR modifies some build requirements and the readme is updated

add test
@aparajit-pratap
Copy link
Contributor

@mjkkirschner can we verify that the ToString calls on these Revit/AutoCAD objects are throwing the exceptions and that they are the reason for the crash?

@mjkkirschner
Copy link
Member Author

I can try to reproduce locally with Revit, but the stack trace seems to imply that:
#10816

@mjkkirschner
Copy link
Member Author

Screen Shot 2020-07-30 at 5 01 25 PM

crashed before - after seems to execute ok.

@mjkkirschner mjkkirschner merged commit 15bdd51 into DynamoDS:master Jul 30, 2020
@mjkkirschner mjkkirschner deleted the protectCLRtostring branch July 30, 2020 21:02
@aparajit-pratap
Copy link
Contributor

aparajit-pratap commented Jul 30, 2020

@mjkkirschner it looks like the exception was occurring for only certain parameters and not all. In cases, where the exception is being thrown and caught, there should be empty strings, am I right?

@mjkkirschner
Copy link
Member Author

yep!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants