Skip to content

Logging enhancement - #160

Merged
Rider-Linden merged 2 commits into
developfrom
rider/logging
Sep 23, 2026
Merged

Rider-Linden merged 2 commits into
developfrom
rider/logging

Conversation

@Rider-Linden

Copy link
Copy Markdown
Collaborator

Adds configurable logging levels, ERROR, WARN, INFO, DEBUG, and TRACE. Settable from the plugin config window.
Adds full JSON-RPC message dump in when in TRACE.

Fixes #159

…faults to `INFO`. Also add TRACE level, in trace we dump the bodies of all json rpc message.
Copilot AI lite review requested due to automatic review settings September 22, 2026 22:33
@Rider-Linden Rider-Linden added the enhancement New feature or request label Sep 22, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Unresolved moderate issues remain around TRACE serialization error handling and invalid logging-level fallback behavior.

Review effort: Lite
Findings: None

What changed in this PR

Adds configurable ERROR/WARN/INFO/DEBUG/TRACE logging, including TRACE-level JSON-RPC payload dumps and configuration UI support.

Changes:

  • Adds shared severity filtering and runtime configuration updates.
  • Routes WebSocket and preprocessor logs through the shared logger.
  • Updates interfaces, documentation, tests, and package scripts.
File Summary
src/​utils.ts Integrates configured logging. Nit (1 vote): add fallback tests for missing/invalid values resetting logging to INFO.
src/​test/​suite/​logger.test.ts Adds logger behavior tests.
src/​synchservice.ts Uses the shared WebSocket logger.
src/​shared/​logger.ts Implements filtering. Moderate (1 vote): WARNING should be removed or documented; invalid values must fall back to INFO.
src/​server/​nodehost.ts Adds trace logger support.
src/​scriptsync.ts Routes preprocessor logs through the shared logger.
src/​interfaces/​configinterface.ts Adds the logging configuration key.
src/​extension.ts Initializes logging configuration.
README.md Documents logging settings.
packages/​sl-script-preprocessor/​src/​interfaces.ts Extends the preprocessor logger interface.
packages/​sl-ide-ws-client/​src/​jsonrpcclient.ts Adds TRACE payload dumps. Three moderate findings (1 vote each): request, notification, and response serialization occur before the existing error boundary and can throw for non-serializable values.
packages/​sl-ide-ws-client/​src/​events.ts Extends the WebSocket logger interface.
package.json Adds the logging setting and test updates.
doc/​USER_GUIDE.md Documents logging configuration.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Source compatibility regressions, inaccurate or incomplete TRACE output, and missing configuration integration tests remain unresolved.

Review effort: Balanced
Findings: None

Previously missed (2)

In code that hasn't changed since last review

Medium severity Preserve string-only WsLogger callback compatibility

packages/​sl-ide-ws-client/​src/​events.ts:59

Changing the existing debug/info/warn/error callback parameters from string to string | (() => string) is a source-breaking change for hosts that implement WsLogger with string-only callbacks. The new call sites use thunks only for trace, so keep the existing four signatures string-based and use the thunk union only for the new trace callback (or provide an adapter/overload).

Medium severity Preserve string-only PreprocessorLogger callback compatibility

packages/​sl-script-preprocessor/​src/​interfaces.ts:40

Changing the existing logger callbacks from string to string | (() => string) breaks existing PreprocessorLogger implementations whose methods accept only strings. None of the existing debug/info/warn/error call sites need lazy thunks; retain their string signatures and reserve the thunk union for the new trace callback, or add an explicit compatibility adapter.

@Rider-Linden
Rider-Linden merged commit 829ec95 into develop Sep 23, 2026
4 checks passed
@Rider-Linden
Rider-Linden deleted the rider/logging branch September 23, 2026 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Configurable Log Level Filtering

2 participants