Skip to content

fix: use structured logging for debug/info methods#4767

Merged
leaanthony merged 10 commits intov3-alphafrom
fix/build-errors
Dec 11, 2025
Merged

fix: use structured logging for debug/info methods#4767
leaanthony merged 10 commits intov3-alphafrom
fix/build-errors

Conversation

@leaanthony
Copy link
Copy Markdown
Member

@leaanthony leaanthony commented Dec 11, 2025

Summary

  • Changed debug() and info() methods to pass args directly to Logger.Debug/Info instead of using fmt.Sprintf()
  • This fixes "call has arguments but no formatting directives" errors since callers were using slog-style key-value pairs

Test plan

  • CI pipeline passes
  • Verify no "formatting directive" errors in go vet

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Changed

    • Default build mode is now production; set DEV=true for dev builds (breaking).
  • New Features

    • CLI now reports NVIDIA driver info on Linux.
  • Bug Fixes

    • iOS build failures from service stubs fixed.
    • WebKit on Wayland with NVIDIA GPUs now avoids a crash by auto-disabling a renderer.
    • Structured-logging formatting errors fixed.
  • Chores

    • Removed accidental debug prints and standardized structured logging.

✏️ Tip: You can customize this high-level summary in your review settings.

The debug() and info() methods were using fmt.Sprintf() which expects
printf-style format directives, but callers were using slog-style
key-value pairs. Changed to pass args directly to Logger.Debug/Info
which properly handles structured logging.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages bot commented Dec 11, 2025

Deploying wails with  Cloudflare Pages  Cloudflare Pages

Latest commit: ed39497
Status: ✅  Deploy successful!
Preview URL: https://db01592d.wails.pages.dev
Branch Preview URL: https://fix-build-errors.wails.pages.dev

View logs

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Dec 11, 2025

Walkthrough

Replaces many ad-hoc fmt.* debug prints with structured logging or removals, adds NVIDIA/Wayland startup handling and Linux NVIDIA driver detection, adds an iOS Go build constraint, updates the unreleased changelog, and removes a submodule pointer. No exported/public API signatures changed.

Changes

Cohort / File(s) Summary
Changelog
v3/UNRELEASED_CHANGELOG.md
Updated changelog entries: production-build default, NVIDIA/WebKit/Wayland fix, logging fixes, iOS/service/build fixes.
Logging removals / structured logging
v3/pkg/application/application.go, v3/pkg/application/application_darwin.go, v3/pkg/application/application_debug.go, v3/pkg/application/application_ios.go, v3/pkg/application/init_android.go, v3/pkg/application/init_ios.go, v3/pkg/application/mainthread_android.go, v3/pkg/application/mainthread_ios.go, v3/pkg/application/systemtray_windows.go, v3/pkg/application/webview_window.go, v3/pkg/application/webview_window_ios.go, v3/pkg/application/webview_window_windows.go
Removed plain fmt.* debug prints and converted some messages to structured logging (message + key/value pairs); no control-flow or public API changes.
Linux Wayland / NVIDIA handling
v3/pkg/application/application_linux.go
New init logic disables WebKit DMA-BUF renderer on Wayland when an NVIDIA GPU is present (sets WEBKIT_DISABLE_DMABUF_RENDERER=1), plus helper isNVIDIAGPU().
Linux NVIDIA driver info
v3/internal/doctor/doctor_linux.go
Adds NVIDIA driver detection (reads /sys/module/nvidia/version and optional srcversion) and exposes "NVIDIA Driver" in doctor info.
Asset server logging removal
v3/internal/assetserver/assetserver_webview.go
Removed debug prints related to iOS/wails:// request debug and host mismatch messages; behavior unchanged.
iOS build constraint
v3/internal/commands/build_assets/ios/main.m
Added //go:build ios build constraint at file top.
Submodule pointer removal
wails-mimetype-migration
Deleted a single-line submodule commit reference.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Files to inspect closely:
    • v3/pkg/application/application_linux.go — confirm environment changes are safe and correctly scoped to Wayland/NVIDIA cases.
    • v3/internal/doctor/doctor_linux.go — validate file reads and fallbacks for missing sysfs entries.
    • v3/pkg/application/application.go — ensure async logging goroutine and panic handling remain correct after logging changes.
    • Platform-specific files (*_ios.go, *_android.go, *_windows.go) — verify structured log key consistency and that removed prints weren't relied on for debugging-critical flows.
    • v3/internal/commands/build_assets/ios/main.m — confirm build constraint placement is correct.

Possibly related PRs

Suggested labels

Bug, go, v3-alpha, size:M, lgtm

Suggested reviewers

  • atterpac

Poem

🐰 Hop-hop, the logs now softly hum,

Quieted prints and tidy drum.
Flags set right for Apple and NVIDIA,
Submodule gone, changelog shiny. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description provides a clear summary of changes and a test plan, but does not follow the required template: missing issue link, type of change checkbox, test configuration details, and most checklist items. Add the issue number (Fixes #), select the bug fix checkbox, provide test configuration from 'wails doctor', and complete the required checklist items.
Docstring Coverage ⚠️ Warning Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: use structured logging for debug/info methods' accurately describes the main change: converting debug/info methods to use structured logging instead of fmt.Sprintf formatting.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/build-errors

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b1c5bb6 and ed39497.

📒 Files selected for processing (4)
  • v3/UNRELEASED_CHANGELOG.md (1 hunks)
  • v3/internal/assetserver/assetserver_webview.go (0 hunks)
  • v3/internal/doctor/doctor_linux.go (1 hunks)
  • v3/pkg/application/application_linux.go (1 hunks)
💤 Files with no reviewable changes (1)
  • v3/internal/assetserver/assetserver_webview.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • v3/UNRELEASED_CHANGELOG.md
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: fbbdev
Repo: wailsapp/wails PR: 4066
File: v3/pkg/application/messageprocessor_call.go:174-174
Timestamp: 2025-02-13T01:05:02.267Z
Learning: When handling JSON marshaling errors in Wails v3, the error message from json.Marshal provides sufficient debugging context. Logging raw data is unnecessary and could make logs harder to read.
📚 Learning: 2025-10-17T23:16:11.570Z
Learnt from: Sammy-T
Repo: wailsapp/wails PR: 4570
File: v2/internal/frontend/desktop/linux/window_webkit6.go:97-108
Timestamp: 2025-10-17T23:16:11.570Z
Learning: For webkit_6/GTK4 builds in v2/internal/frontend/desktop/linux/window_webkit6.go, GTK widget creation should not be wrapped in invokeOnMainThread. The activation mechanism (activateWg + onActivate export) already handles thread safety, and additional wrapping would cause issues.

Applied to files:

  • v3/pkg/application/application_linux.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Run Go Tests v3 (macos-latest, 1.24)
  • GitHub Check: Run Go Tests v3 (windows-latest, 1.24)
  • GitHub Check: Run Go Tests v3 (ubuntu-latest, 1.24)
  • GitHub Check: Analyze (go)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: Cloudflare Pages
🔇 Additional comments (5)
v3/internal/doctor/doctor_linux.go (3)

6-7: LGTM!

The added imports are necessary for the NVIDIA driver detection functionality.


15-17: LGTM!

Clean integration of NVIDIA driver detection into the doctor info output.


22-36: LGTM!

The NVIDIA driver detection is well-implemented with appropriate error handling. The function gracefully handles both missing driver scenarios and optional srcversion information. Reading from sysfs paths is safe, and the formatted output is clear.

v3/pkg/application/application_linux.go (2)

38-46: LGTM! Well-documented workaround for a known WebKitGTK issue.

The conditional environment variable setup is appropriate and defensive. The checks ensure the workaround only applies when needed (Wayland + NVIDIA) and respects any existing user configuration. The bug reference adds helpful context.


48-55: LGTM! Reliable detection method for NVIDIA proprietary driver.

The implementation correctly detects the presence of the NVIDIA proprietary driver by checking for the loaded kernel module. The function is appropriately scoped and used only for the Wayland workaround.

Note: The function specifically detects the proprietary driver (not nouveau), which aligns with the workaround's purpose since the DMA-BUF issue is specific to NVIDIA's proprietary drivers.


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

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

@github-actions
Copy link
Copy Markdown
Contributor

⚠️ Missing Changelog Update

Hi @leaanthony, please update v3/UNRELEASED_CHANGELOG.md with a description of your changes.

This helps us keep track of changes for the next release.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2e396bd and cd6ed78.

📒 Files selected for processing (2)
  • v3/UNRELEASED_CHANGELOG.md (1 hunks)
  • v3/pkg/application/application.go (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: fbbdev
Repo: wailsapp/wails PR: 4066
File: v3/pkg/application/messageprocessor_call.go:174-174
Timestamp: 2025-02-13T01:05:02.267Z
Learning: When handling JSON marshaling errors in Wails v3, the error message from json.Marshal provides sufficient debugging context. Logging raw data is unnecessary and could make logs harder to read.
🧬 Code graph analysis (1)
v3/pkg/application/application.go (1)
v3/pkg/services/log/log.go (2)
  • Info (20-20)
  • Debug (19-19)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Run Go Tests v3 (macos-latest, 1.24)
  • GitHub Check: Run Go Tests v3 (windows-latest, 1.24)
  • GitHub Check: Run Go Tests v3 (ubuntu-latest, 1.24)
  • GitHub Check: Analyze (go)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: Cloudflare Pages
🔇 Additional comments (1)
v3/UNRELEASED_CHANGELOG.md (1)

26-27: LGTM!

The changelog entries accurately document the fixes in this PR and follow the established format.

Comment thread v3/pkg/application/application.go
Comment thread v3/pkg/application/application.go
leaanthony and others added 3 commits December 12, 2025 06:26
Remove emoji debug logs (🔴, 🟢, 🟠, 🔵, 🔥) that were accidentally left in
from the iOS/Android mobile platform support merge. These were development
debugging statements that should not have been included in the final code.

Files cleaned:
- application.go
- application_debug.go
- init_android.go
- init_ios.go
- mainthread_android.go
- mainthread_ios.go
- webview_window.go
- webview_window_ios.go

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add breaking change note: production builds are now default
- Add entry for debug print statement removal

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Convert three debug() calls that were still using printf-style format
strings to slog-style structured logging (key-value pairs):

- systemtray_windows.go: ShellNotifyIcon show/hide failures
- application_darwin.go: window lookup failure

This addresses CodeRabbit review feedback and ensures consistency
with the refactored debug() method.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Convert remaining info() calls that were using printf-style format
strings to slog-style structured logging (key-value pairs):

- application_ios.go: iOS log messages and HandleJSMessage calls
- webview_window_windows.go: WM_SYSKEYDOWN logging
- application.go: handleWindowMessage and handleWebViewRequest logging

Also removed debug fmt.Printf statements from handleWebViewRequest.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…OS platforms

Go's toolchain tries to process .m (Objective-C) files when they're in a
directory with Go files. Adding a //go:build ios tag tells Go to only
process this file when building for iOS, matching how darwin .m files
handle this (e.g., //go:build darwin && !ios).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@leaanthony leaanthony enabled auto-merge (squash) December 11, 2025 19:49
The iOS merge added a submodule reference without a corresponding
.gitmodules file, causing Cloudflare and other CI systems to fail
with "No url found for submodule path" errors.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@leaanthony leaanthony disabled auto-merge December 11, 2025 20:36
leaanthony and others added 2 commits December 12, 2025 08:39
…crashes

WebKitGTK has a known issue with the DMA-BUF renderer on NVIDIA proprietary
drivers running Wayland, causing "Error 71 (Protocol error)" crashes.

This fix automatically detects NVIDIA GPUs (via /sys/module/nvidia) and sets
WEBKIT_DISABLE_DMABUF_RENDERER=1 when running on Wayland.

Also removes leftover debug print statements from mobile platform merge.

See: https://bugs.webkit.org/show_bug.cgi?id=262607

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Shows NVIDIA driver version and srcversion in doctor output to help
diagnose Wayland/NVIDIA compatibility issues.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@sonarqubecloud
Copy link
Copy Markdown

@leaanthony leaanthony enabled auto-merge (squash) December 11, 2025 21:44
@leaanthony leaanthony disabled auto-merge December 11, 2025 21:50
@leaanthony leaanthony enabled auto-merge (squash) December 11, 2025 21:50
@leaanthony leaanthony disabled auto-merge December 11, 2025 22:03
@leaanthony leaanthony merged commit c4b614c into v3-alpha Dec 11, 2025
55 checks passed
@leaanthony leaanthony deleted the fix/build-errors branch December 11, 2025 22:03
@coderabbitai coderabbitai bot mentioned this pull request Jan 4, 2026
15 tasks
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.

1 participant