Don't log yarn exceptions as error but warning

[Boy132]

Those are "soft" errors and can be ignored most of the time.

[coderabbitai]

📝 Walkthrough

Walkthrough

Replaced exception reporting in PluginService::buildAssets by importing the Log facade and logging the exception message with Log::warning(...) instead of calling report(...).

Changes

Cohort / File(s)	Summary
Error Handling & Logging app/Services/Helpers/PluginService.php	Imported Log facade and replaced report($exception) with Log::warning($exception->getMessage()) inside the buildAssets catch block.

Pre-merge checks

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: converting yarn exception logging from error to warning level.
Description check	✅ Passed	The description is related to the changeset, providing context that these are soft errors that can be ignored, justifying the logging level change.

---

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dfdcc68 and 28c5fc4.

📒 Files selected for processing (1)

• app/Services/Helpers/PluginService.php (2 hunks)

🔇 Additional comments (2)

app/Services/Helpers/PluginService.php (2)

19-19: LGTM!

The Log facade import is correctly added and properly positioned in the imports list.

---

268-268: Verify the impact on error monitoring and tracking.

The change from report($exception) to Log::warning() means yarn build failures will no longer be sent to error tracking services (e.g., Sentry, Bugsnag). While this reduces noise for transient issues, it might also hide legitimate problems like:

• Missing dependencies
• JavaScript/CSS syntax errors
• Build configuration issues

Confirm that downgrading all yarn exceptions to warnings aligns with your team's error monitoring strategy, especially since failed builds return false but don't raise alerts.

Note: The context array ['exception' => $exception] is good practice—it preserves the full exception details in logs.

---

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.}

[coderabbitai]

Actionable comments posted: 0

Caution

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

⚠️ Outside diff range comments (1)

app/Services/Helpers/PluginService.php (1)

263-269: Significant loss of debugging context and observability.

While downgrading yarn errors to warnings is reasonable, the current implementation loses critical debugging information:

1. Lost stack traces: $exception->getMessage() only captures the message string, discarding the stack trace that helps diagnose issues in production.

2. Bypassed exception handlers: Laravel's report() sends exceptions to configured handlers (Sentry, Bugsnag, etc.) for monitoring and alerting. Log::warning() bypasses these entirely, potentially breaking observability pipelines.

3. Inconsistent patterns: Lines 182 and 488 still use report($exception), creating inconsistent exception handling across this service.

Consider these alternatives:

• Option 1 (Recommended): Log the full exception object while keeping the warning level:

  Log::warning('Yarn build failed (non-critical)', ['exception' => $exception]);

• Option 2: Report with context to indicate lower severity:

  report($exception)->context(['severity' => 'warning', 'critical' => false]);

• Option 3: Configure your exception handler to treat specific exception types or contexts differently rather than bypassing report() entirely.

🔎 Recommended fix preserving debugging context

-            Log::warning($exception->getMessage());
+            Log::warning('Yarn asset build failed (non-critical)', [
+                'exception' => $exception,
+                'message' => $exception->getMessage(),
+            ]);

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1d5ace3 and dfdcc68.

📒 Files selected for processing (1)

• app/Services/Helpers/PluginService.php (2 hunks)

🔇 Additional comments (1)

app/Services/Helpers/PluginService.php (1)

19-19: Import added for logging facade.

The Log facade import is correctly added to support the warning logging functionality.