Add trace logging to empty catch blocks in CommandDiscovery#27621
Open
euxaristia wants to merge 2 commits into
Open
Add trace logging to empty catch blocks in CommandDiscovery#27621euxaristia wants to merge 2 commits into
CommandDiscovery#27621euxaristia wants to merge 2 commits into
Conversation
Two catch blocks in `LookupCommandInfo` were silently swallowing all exceptions from `PreCommandLookupAction` invocations and module auto-discovery. Log these failures via `discoveryTracer` so they are visible in diagnostic traces.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds trace logging in CommandDiscovery to avoid silently swallowing exceptions during command lookup, improving diagnosability when tracing is enabled.
Changes:
- Log exceptions thrown by
PreCommandLookupActioninstead of silently ignoring them. - Log exceptions thrown during module auto-discovery instead of silently ignoring them.
| catch (Exception) | ||
| catch (Exception e) | ||
| { | ||
| discoveryTracer.TraceError("PreCommandLookupAction threw: {0}", e.Message); |
| catch (Exception) | ||
| catch (Exception e) | ||
| { | ||
| discoveryTracer.TraceError("Module auto-discovery threw: {0}", e.Message); |
Use the exception object directly (which calls ToString for type+message+stack) instead of just e.Message, and include the command name being looked up.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Summary
Add
discoveryTracer.TraceError()calls to two empty catch blocks inCommandDiscovery.LookupCommandInfothat were silently swallowing all exceptions.PR Context
Two catch blocks — one for
PreCommandLookupActioninvoker exceptions and one for module auto-discovery exceptions — were catchingExceptionwith an empty body. This hides failures from diagnostic traces and makes debugging difficult. Both handlers are intentional (external event handlers should not crash command lookup, and module discovery failures should degrade gracefully), but the exceptions should still be logged when tracing is enabled.PR Checklist