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

[DUI3-293] logging poc bindings and bridge #3515

Merged
merged 22 commits into from
Jun 21, 2024

Conversation

JR-Morgan
Copy link
Member

@JR-Morgan JR-Morgan commented Jun 17, 2024

Important changes

  • Added TopLevelExceptionHandler service.
    • This class provides the logging and handling behaviour (toast notification) for all unhandled exceptions that reach a "top level" entry point (e.g. UI or API Event callback function)
    • We should use this for all API events (like OnDocumentChange) and is used in the Bridge for Bindings.
    • If any exception reaches this high, we consider this an unexpected error, and will be logged as such.
    • Exception handling should be performed lower down if the error is recoverable or expected.
  • Cleaned up a couple things in the Bindings, most notably r.e. thread safety

TODO:

  • Test some of the async Action Block changes in CEF 65 (Revit 2021) <- not easy, agreed with Dim that we can do this later
  • Figure out the best practice for logging (ILogger<T> or ILoggerFactory) https://spockle.atlassian.net/browse/DUI3-389
  • Ask Ian about circular deps
  • Unwrap target invocation of sync binding calls (atleast for the execute method return) <- I've done it slightly differently, I'm re-wrapping with a better message
  • Do one last test in all connectors, injecting some exceptions in a few places...
  • Double check logs appear in sync with structured params
  • Check that we aren't somehow changing threads for the async overloads of the toplevel catch <- impossible for no async stuff, we don't do any async

@JR-Morgan JR-Morgan changed the title [Dui3-293] logging poc bindings and bridge [DUI3-293] logging poc bindings and bridge Jun 17, 2024
@JR-Morgan JR-Morgan marked this pull request as draft June 17, 2024 15:19
Copy link
Member

@oguzhankoral oguzhankoral left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preliminary questions and comments. I liked it overall as we walk through before ❤️

@JR-Morgan JR-Morgan requested a review from BovineOx June 19, 2024 16:56
@JR-Morgan JR-Morgan marked this pull request as ready for review June 19, 2024 17:07
@JR-Morgan JR-Morgan merged commit 483fe7d into dui3/alpha Jun 21, 2024
33 checks passed
@JR-Morgan JR-Morgan deleted the DUI3-293-logging-poc-bindings-and-bridge branch June 21, 2024 12:43
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.

3 participants