Skip to content

Conversation

@GeKtvi
Copy link
Contributor

@GeKtvi GeKtvi commented Jul 6, 2025

Added simple message logger to display logs in message boxes. Helps avoid silent failures by showing unhandled exceptions and important warnings to users.

@brinkdinges
Copy link
Member

I like the idea of this, silent problems that are swallowed are annoying for the user. But isn't showing a message box an implementation detail that we should leave to the developer? Could this also be an event? Most users don't need a stack trace.

I've never worked on logging on SolidDNA and I don't even know if it works, see #44. I also don't know if it shares data between two add-ins, which would be problematic. So I can't properly judge this PR yet.

Questions:

  • What is a scope?
  • Why do we need a MessageLogScope and why is it disposable?
  • Why does it need stacks and a hashset? Why does the order matter?

Remarks:

  • Please create a PR with just these changes.
  • Please create a single class per file.
  • Please remove the discards when you don't use the return value of a method.

@GeKtvi
Copy link
Contributor Author

GeKtvi commented Jul 14, 2025

But isn't showing a message box an implementation detail that we should leave to the developer?

I agree. I can add a setting property to toggle stack trace visibility.

Could this also be an event?

I’m not sure which event you’re referring to. Could you clarify?

Most users don't need a stack trace.

Agreed—I’ll make it optional.

I've never worked on logging on SolidDNA and I don't even know if it works, see #44.

It’s straightforward—similar to Microsoft’s logging library. SolidDNA’s static Logger class supports pluggable loggers (currently file-based, and this message logger soon). I’ve used it in two internal add-ins by wrapping it to MS logger and use it in DI container, and it works well.

I also don't know if it shares data between two add-ins, which would be problematic.

It doesn’t share data between add-ins. Using different SolidDNA versions only overlap would be duplicate console messages. For message boxes, conflicts are rare (e.g., if errors occur in shared SolidDNA code). Even then, duplicate messages would likely be the least of the user’s concerns. In production, log level below Error is not used.

What is a scope?

A scope groups all messages logged during its lifetime (from creation to disposal) into a single message box. This avoids spamming users with separate popups for each inner method’s warnings/errors.

Why do we need a MessageLogScope and why is it disposable?

This follows standard patterns (e.g., MS logging/DI). IDisposable marks an object’s lifetime—clean and brace-free with new using.

Why does it need stacks and a hashset? Why does the order matter?

Order ensures messages display chronologically. The stack tracks nested scopes, the hashset prevents duplicates.

Please create a PR with just these changes.

As mentioned in other PRs: if you merge the Command Manager PR first, this won’t be needed. Otherwise, I can create a fresh PR.

Please create a single class per file.

I nested these classes to avoid cluttering autocomplete—they’re only used by the parent class. If you prefer separate files, I’ll move them.

Please remove the discards when you don't use the return value of a method.

Understood. Just let me know whether to proceed with this PR or a new one, and I’ll apply fixes immediately.

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