Increase path field max size and improve telemetry errors

[DropSnorz]

No description provided.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

• 🔍 Trigger a full review

Walkthrough

The pull request refactors error logging to extract nested exception details and adds a summary parameter to telemetry tracking. It also applies database column length constraints (512 characters) to file path fields across plugin model classes and introduces unit tests for telemetry data sanitization.

Changes

Cohort / File(s)	Summary
Error Logging & Exception Handling owlplug-client/src/main/java/com/owlplug/core/components/TaskRunner.java	Imports NestedExceptionUtils and modifies exception logging to extract the most specific cause message; adds fallback message "No summary available" when no error information exists
Task Bar Controller & Telemetry Mapping owlplug-client/src/main/java/com/owlplug/core/controllers/TaskBarController.java	Adds new summary parameter to setErrorLog() method signature; remaps telemetry payload to send summary instead of content to the tracking service
Telemetry Service owlplug-client/src/main/java/com/owlplug/core/services/TelemetryService.java	Changes sanitize() visibility from private to package-private; adds overloaded sanitize(Map, int maxLength) to support configurable truncation length
Plugin Model Database Constraints owlplug-client/src/main/java/com/owlplug/plugin/model/Plugin.java, PluginDirectory.java, PluginFootprint.java	Adds @Column(length = 512) JPA annotations to path fields (path, scanDirectoryPath) across plugin model classes to enforce database column size limits
Telemetry Service Tests owlplug-client/src/test/java/com/owlplug/core/services/TelemetryServiceTest.java	New test class with 5 unit tests covering path redaction (Unix/Windows), class name preservation, value truncation with ellipsis, and multi-entry independence

Sequence Diagram(s)

sequenceDiagram
    participant TaskRunner as TaskRunner
    participant TBC as TaskBarController
    participant TS as TelemetryService
    participant Telemetry as Telemetry Backend
    
    TaskRunner->>TaskRunner: Task execution fails<br/>catch exception
    TaskRunner->>TaskRunner: Extract most specific<br/>cause via NestedExceptionUtils
    TaskRunner->>TBC: setErrorLog(task, title, content, summary)
    TBC->>TBC: Update UI & show error dialog
    TBC->>TS: Prepare telemetry payload<br/>with summary field
    TS->>TS: sanitize(params, maxLength)<br/>Redact paths & truncate
    TS->>Telemetry: Send sanitized payload

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• Add simple permission checks before plugin uninstall #390: Modifies TaskRunner's failure-path error logging to pass different exception details to taskBarController.setErrorLog, directly related to the nested exception handling changes here.
• Add Telemetry service #356: Modifies both TelemetryService and TaskBarController signatures and behavior, overlapping with the telemetry refactoring and method signature updates in this PR.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	No pull request description was provided by the author, making it impossible to assess whether it relates to the changeset.	Add a description explaining the motivation for increasing path field constraints and the telemetry error improvements.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the two main changes: increasing path field max size and improving telemetry error handling, which aligns with the core modifications across multiple files.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[DropSnorz]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

owlplug-client/src/main/java/com/owlplug/core/services/TelemetryService.java (1)

113-125: Truncation before redaction may leave partial paths unredacted.

The current order truncates the value first (lines 116-118), then applies path redaction (line 121). If a path spans the truncation boundary, it may be cut in a way that no longer matches the redaction regex, potentially leaking partial sensitive paths.

Also, if entry.getValue() returns null, value.length() on line 116 will throw a NullPointerException.

Consider:

1. Applying redaction before truncation, or
2. Adding a null check for the value

Proposed fix

  void sanitize(Map<String, String> params, int maxLength) {
    for (Map.Entry<String, String> entry : params.entrySet()) {
      String value = entry.getValue();
+     if (value == null) {
+       continue;
+     }
+
+     // Redact absolute paths first (Unix & Windows)
+     value = value.replaceAll("([A-Za-z]:\\\\\\\\[^\\s]+)|(/[^\\s]+)", "<path>");
+
      if (value.length() > maxLength) {
        value = value.substring(0, maxLength) + "…";
      }
-
-     // Redact absolute paths (Unix & Windows)
-     value = value.replaceAll("([A-Za-z]:\\\\\\\\[^\\s]+)|(/[^\\s]+)", "<path>");
      entry.setValue(value);
-
    }
  }

🤖 Fix all issues with AI agents

In `@owlplug-client/src/main/java/com/owlplug/core/components/TaskRunner.java`:
- Around line 118-120: In TaskRunner replace the direct call to
NestedExceptionUtils.getMostSpecificCause(ex).getMessage() with a null-safe
extraction: retrieve the most specific cause via
NestedExceptionUtils.getMostSpecificCause(ex), check if that cause is null or
cause.getMessage() is null, and use a safe fallback (e.g., empty string or
cause.getClass().getSimpleName()) for the summary; update the code paths that
build the summary (the block calling StringUtils.getStackTraceAsString(ex) and
NestedExceptionUtils.getMostSpecificCause(ex).getMessage()) to use this
null-safe message.

🧹 Nitpick comments (1)

owlplug-client/src/test/java/com/owlplug/core/services/TelemetryServiceTest.java (1)

8-10: Test instantiation bypasses Spring context.

TelemetryService is a Spring @Service with @PostConstruct initialization and autowired dependencies. Directly instantiating it via new TelemetryService() works for the current sanitize() tests because that method doesn't rely on injected fields, but this approach is fragile if the implementation changes.

Consider adding a test for null values in the map to align with the null-safety concern in sanitize():

Suggested additional test case

`@Test`
public void testShouldHandleNullValues() {
  Map<String, String> params = new HashMap<>();
  params.put("key", null);

  telemetryService.sanitize(params);

  // Verify no exception and value unchanged or handled gracefully
}