[DNM] Remove Dispatcher unhandled exception handler since it's already handled in DynamoCore#3029
[DNM] Remove Dispatcher unhandled exception handler since it's already handled in DynamoCore#3029aparajit-pratap wants to merge 5 commits intoDynamoDS:masterfrom
Conversation
aparajit-pratap
changed the title
aparajit-pratap
changed the title
| finally | ||
| { | ||
| args.Handled = true; | ||
| DynamoRevitApp.DynamoButtonEnabled = true; |
There was a problem hiding this comment.
what about the button ?
There was a problem hiding this comment.
The button is reenabled even without this.
There was a problem hiding this comment.
Is it somewhere in the view.CLose events ?
THere would be a chance that if an exception is thrown in the exception handler, then the button close will not be called at all
There was a problem hiding this comment.
Yes, it is re-enabled in OnDynamoViewClosed here.
There was a problem hiding this comment.
But why the API breaking change so late, in a minor build (3.0.2 vs 3.0.0), and why after Revit API freeze?
This would require more testing on Revit side, that various CER scenarios still work and we are not missing crash reports, possibly even more code changed.
Is this API change in 3.0.2 mandatory or can we revert it, or make other builds based on 3.0.0 with only the critical fixes? @QilongTang FYI
There was a problem hiding this comment.
@Mikhinja this is not an API breaking change - the API change is in this PR: #3032
This PR is about removing the API call entirely in D4R and moving it to DynamoCore instead so other host integrations can benefit from it without needing to call it explicitly in their integration code.
Having said that, the Dynamo team is only experimenting with this so far and we are not ready to merge this for global launch, which is why I've marked this PR as DNM. We will continue refining this approach post 3.0.x.
Note that PR #3032 does need to be merged into D4R when it begins to consume 3.0.1 or 3.0.2 and higher.
There was a problem hiding this comment.
Yes, that change is what I was actually referring to. I merged it together with Dynamo 3.0.2.
4 participants
Purpose
This PR depends on DynamoDS/Dynamo#14840 first being merged into Dynamo 3.0.3. If not for 3.0.3, it can wait until the Dynamo PR is merged into 3.1.
It skips certain steps in the Dispatcher's unhandled exception handler if the corresponding handler is already invoked in DynamoViewModel.Correction: The Dispatcher's unhandled exception event handler has been moved to DynamoViewModel in DynamoDS/Dynamo#14840. It's no longer needed here.
Declarations
Check these if you believe they are true
*.resxfilesReviewers
@pinzart90
FYIs
(FILL ME IN, Optional) Names of anyone else you wish to be notified of