fix: apply review feedback across docs, filtering, webkit and infra
- commit
- ff377bb7687582418aff03fae32025bfa7446680
- author
- brice · · 2026-02-07 06:39 UTC
- parent
- e8d3633
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
16 files · +145 -73
M docs/config/index.md +2 -0
| @@ -439,12 +439,14 @@ gstreamer_debug_level = 0 # Increase to 3-5 for debugging | |||
| 439 | 439 | ``` | |
| 440 | 440 | ||
| 441 | 441 | **Diagnostic mode toggle path:** | |
| 442 | + | ||
| 442 | 443 | ```toml | |
| 443 | 444 | # ~/.config/dumber/config.toml | |
| 444 | 445 | [media] | |
| 445 | 446 | show_diagnostics = true | |
| 446 | 447 | gstreamer_debug_level = 3 | |
| 447 | 448 | ``` | |
| 449 | + | ||
| 448 | 450 | Set `show_diagnostics = false` and `gstreamer_debug_level = 0` to return to normal mode. | |
| 449 | 451 | ||
| 450 | 452 | **Diagnostics CLI:** | |
| @@ -439,12 +439,14 @@ gstreamer_debug_level = 0 # Increase to 3-5 for debugging | |||||
| 439 | ``` | 439 | ``` | ||
| 440 | 440 | ||||
| 441 | **Diagnostic mode toggle path:** | 441 | **Diagnostic mode toggle path:** | ||
| 442 | + | ||||
| 442 | ```toml | 443 | ```toml | ||
| 443 | # ~/.config/dumber/config.toml | 444 | # ~/.config/dumber/config.toml | ||
| 444 | [media] | 445 | [media] | ||
| 445 | show_diagnostics = true | 446 | show_diagnostics = true | ||
| 446 | gstreamer_debug_level = 3 | 447 | gstreamer_debug_level = 3 | ||
| 447 | ``` | 448 | ``` | ||
| 449 | + | ||||
| 448 | Set `show_diagnostics = false` and `gstreamer_debug_level = 0` to return to normal mode. | 450 | Set `show_diagnostics = false` and `gstreamer_debug_level = 0` to return to normal mode. | ||
| 449 | 451 | ||||
| 450 | **Diagnostics CLI:** | 452 | **Diagnostics CLI:** | ||
M docs/reference/troubleshooting.md +5 -0
| @@ -30,23 +30,28 @@ dumber doctor --media # GStreamer/VA-API only | |||
| 30 | 30 | **Solution:** | |
| 31 | 31 | 1. Run `dumber doctor --media` | |
| 32 | 32 | 2. Enable diagnostic mode for startup media warnings: | |
| 33 | + | ||
| 33 | 34 | ```toml | |
| 34 | 35 | # ~/.config/dumber/config.toml | |
| 35 | 36 | [media] | |
| 36 | 37 | show_diagnostics = true | |
| 37 | 38 | gstreamer_debug_level = 3 | |
| 38 | 39 | ``` | |
| 40 | + | ||
| 39 | 41 | 3. Check hardware decoding: | |
| 42 | + | ||
| 40 | 43 | ```toml | |
| 41 | 44 | # ~/.config/dumber/config.toml | |
| 42 | 45 | [media] | |
| 43 | 46 | hardware_decoding = "auto" # Try "disable" if issues persist | |
| 44 | 47 | ``` | |
| 48 | + | ||
| 45 | 49 | 4. Install VA-API drivers: | |
| 46 | 50 | - AMD: `libva-mesa-driver` | |
| 47 | 51 | - Intel: `intel-media-driver` | |
| 48 | 52 | - NVIDIA: `libva-nvidia-driver` | |
| 49 | 53 | 5. After diagnosing, disable diagnostic mode for quieter daily runs: | |
| 54 | + | ||
| 50 | 55 | ```toml | |
| 51 | 56 | [media] | |
| 52 | 57 | show_diagnostics = false | |
| @@ -30,23 +30,28 @@ dumber doctor --media # GStreamer/VA-API only | |||||
| 30 | **Solution:** | 30 | **Solution:** | ||
| 31 | 1. Run `dumber doctor --media` | 31 | 1. Run `dumber doctor --media` | ||
| 32 | 2. Enable diagnostic mode for startup media warnings: | 32 | 2. Enable diagnostic mode for startup media warnings: | ||
| 33 | + | ||||
| 33 | ```toml | 34 | ```toml | ||
| 34 | # ~/.config/dumber/config.toml | 35 | # ~/.config/dumber/config.toml | ||
| 35 | [media] | 36 | [media] | ||
| 36 | show_diagnostics = true | 37 | show_diagnostics = true | ||
| 37 | gstreamer_debug_level = 3 | 38 | gstreamer_debug_level = 3 | ||
| 38 | ``` | 39 | ``` | ||
| 40 | + | ||||
| 39 | 3. Check hardware decoding: | 41 | 3. Check hardware decoding: | ||
| 42 | + | ||||
| 40 | ```toml | 43 | ```toml | ||
| 41 | # ~/.config/dumber/config.toml | 44 | # ~/.config/dumber/config.toml | ||
| 42 | [media] | 45 | [media] | ||
| 43 | hardware_decoding = "auto" # Try "disable" if issues persist | 46 | hardware_decoding = "auto" # Try "disable" if issues persist | ||
| 44 | ``` | 47 | ``` | ||
| 48 | + | ||||
| 45 | 4. Install VA-API drivers: | 49 | 4. Install VA-API drivers: | ||
| 46 | - AMD: `libva-mesa-driver` | 50 | - AMD: `libva-mesa-driver` | ||
| 47 | - Intel: `intel-media-driver` | 51 | - Intel: `intel-media-driver` | ||
| 48 | - NVIDIA: `libva-nvidia-driver` | 52 | - NVIDIA: `libva-nvidia-driver` | ||
| 49 | 5. After diagnosing, disable diagnostic mode for quieter daily runs: | 53 | 5. After diagnosing, disable diagnostic mode for quieter daily runs: | ||
| 54 | + | ||||
| 50 | ```toml | 55 | ```toml | ||
| 51 | [media] | 56 | [media] | ||
| 52 | show_diagnostics = false | 57 | show_diagnostics = false | ||
M internal/bootstrap/session.go +9 -4
| @@ -4,6 +4,7 @@ import ( | |||
| 4 | 4 | "context" | |
| 5 | 5 | "errors" | |
| 6 | 6 | "fmt" | |
| 7 | + | stdlog "log" | |
| 7 | 8 | "os" | |
| 8 | 9 | "path/filepath" | |
| 9 | 10 | "strings" | |
| @@ -352,15 +353,19 @@ func markAbruptExits(lockDir string, detectedAt time.Time) ([]string, error) { | |||
| 352 | 353 | if sessionID == "" { | |
| 353 | 354 | continue | |
| 354 | 355 | } | |
| 355 | - | if _, err := os.Stat(shutdownMarkerPath(lockDir, sessionID)); err == nil { | |
| 356 | + | shutdownPath := shutdownMarkerPath(lockDir, sessionID) | |
| 357 | + | if _, err := os.Stat(shutdownPath); err == nil { | |
| 356 | 358 | continue | |
| 357 | 359 | } else if !os.IsNotExist(err) { | |
| 358 | - | return abruptSessions, err | |
| 360 | + | stdlog.Printf("markAbruptExits: stat failed session_id=%s path=%s err=%v", sessionID, shutdownPath, err) | |
| 361 | + | continue | |
| 359 | 362 | } | |
| 360 | - | if _, err := os.Stat(abruptMarkerPath(lockDir, sessionID)); err == nil { | |
| 363 | + | abruptPath := abruptMarkerPath(lockDir, sessionID) | |
| 364 | + | if _, err := os.Stat(abruptPath); err == nil { | |
| 361 | 365 | continue | |
| 362 | 366 | } else if !os.IsNotExist(err) { | |
| 363 | - | return abruptSessions, err | |
| 367 | + | stdlog.Printf("markAbruptExits: stat failed session_id=%s path=%s err=%v", sessionID, abruptPath, err) | |
| 368 | + | continue | |
| 364 | 369 | } | |
| 365 | 370 | ||
| 366 | 371 | payload := []byte(fmt.Sprintf("detected_at=%s\nstartup_marker=%s\n", detectedAt.Format(time.RFC3339Nano), startupPath)) | |
| @@ -4,6 +4,7 @@ import ( | |||||
| 4 | "context" | 4 | "context" | ||
| 5 | "errors" | 5 | "errors" | ||
| 6 | "fmt" | 6 | "fmt" | ||
| 7 | + | stdlog "log" | |||
| 7 | "os" | 8 | "os" | ||
| 8 | "path/filepath" | 9 | "path/filepath" | ||
| 9 | "strings" | 10 | "strings" | ||
| @@ -352,15 +353,19 @@ func markAbruptExits(lockDir string, detectedAt time.Time) ([]string, error) { | |||||
| 352 | if sessionID == "" { | 353 | if sessionID == "" { | ||
| 353 | continue | 354 | continue | ||
| 354 | } | 355 | } | ||
| 355 | - | if _, err := os.Stat(shutdownMarkerPath(lockDir, sessionID)); err == nil { | 356 | + | shutdownPath := shutdownMarkerPath(lockDir, sessionID) |
| 357 | + | if _, err := os.Stat(shutdownPath); err == nil { | |||
| 356 | continue | 358 | continue | ||
| 357 | } else if !os.IsNotExist(err) { | 359 | } else if !os.IsNotExist(err) { | ||
| 358 | - | return abruptSessions, err | 360 | + | stdlog.Printf("markAbruptExits: stat failed session_id=%s path=%s err=%v", sessionID, shutdownPath, err) |
| 361 | + | continue | |||
| 359 | } | 362 | } | ||
| 360 | - | if _, err := os.Stat(abruptMarkerPath(lockDir, sessionID)); err == nil { | 363 | + | abruptPath := abruptMarkerPath(lockDir, sessionID) |
| 364 | + | if _, err := os.Stat(abruptPath); err == nil { | |||
| 361 | continue | 365 | continue | ||
| 362 | } else if !os.IsNotExist(err) { | 366 | } else if !os.IsNotExist(err) { | ||
| 363 | - | return abruptSessions, err | 367 | + | stdlog.Printf("markAbruptExits: stat failed session_id=%s path=%s err=%v", sessionID, abruptPath, err) |
| 368 | + | continue | |||
| 364 | } | 369 | } | ||
| 365 | 370 | ||||
| 366 | payload := []byte(fmt.Sprintf("detected_at=%s\nstartup_marker=%s\n", detectedAt.Format(time.RFC3339Nano), startupPath)) | 371 | payload := []byte(fmt.Sprintf("detected_at=%s\nstartup_marker=%s\n", detectedAt.Format(time.RFC3339Nano), startupPath)) | ||
M internal/infrastructure/config/loader.go +6 -3
| @@ -226,9 +226,12 @@ func normalizePrivacy(config *Config) { | |||
| 226 | 226 | } | |
| 227 | 227 | ||
| 228 | 228 | func normalizeAppearance(config *Config) { | |
| 229 | - | switch config.Appearance.ColorScheme { | |
| 230 | - | case ThemePreferDark, ThemePreferLight, ThemeDefault: | |
| 231 | - | case "": | |
| 229 | + | switch strings.ToLower(strings.TrimSpace(config.Appearance.ColorScheme)) { | |
| 230 | + | case strings.ToLower(ThemePreferDark): | |
| 231 | + | config.Appearance.ColorScheme = ThemePreferDark | |
| 232 | + | case strings.ToLower(ThemePreferLight): | |
| 233 | + | config.Appearance.ColorScheme = ThemePreferLight | |
| 234 | + | case strings.ToLower(ThemeDefault): | |
| 232 | 235 | config.Appearance.ColorScheme = ThemeDefault | |
| 233 | 236 | default: | |
| 234 | 237 | config.Appearance.ColorScheme = ThemeDefault | |
| @@ -226,9 +226,12 @@ func normalizePrivacy(config *Config) { | |||||
| 226 | } | 226 | } | ||
| 227 | 227 | ||||
| 228 | func normalizeAppearance(config *Config) { | 228 | func normalizeAppearance(config *Config) { | ||
| 229 | - | switch config.Appearance.ColorScheme { | 229 | + | switch strings.ToLower(strings.TrimSpace(config.Appearance.ColorScheme)) { |
| 230 | - | case ThemePreferDark, ThemePreferLight, ThemeDefault: | 230 | + | case strings.ToLower(ThemePreferDark): |
| 231 | - | case "": | 231 | + | config.Appearance.ColorScheme = ThemePreferDark |
| 232 | + | case strings.ToLower(ThemePreferLight): | |||
| 233 | + | config.Appearance.ColorScheme = ThemePreferLight | |||
| 234 | + | case strings.ToLower(ThemeDefault): | |||
| 232 | config.Appearance.ColorScheme = ThemeDefault | 235 | config.Appearance.ColorScheme = ThemeDefault | ||
| 233 | default: | 236 | default: | ||
| 234 | config.Appearance.ColorScheme = ThemeDefault | 237 | config.Appearance.ColorScheme = ThemeDefault | ||
M internal/infrastructure/filtering/manager.go +33 -7
| @@ -3,9 +3,11 @@ package filtering | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "encoding/json" | |
| 6 | + | "errors" | |
| 6 | 7 | "fmt" | |
| 7 | 8 | "os" | |
| 8 | 9 | "path/filepath" | |
| 10 | + | "strings" | |
| 9 | 11 | "sync" | |
| 10 | 12 | "sync/atomic" | |
| 11 | 13 | "time" | |
| @@ -24,6 +26,10 @@ const ( | |||
| 24 | 26 | CacheMaxAge = 24 * time.Hour | |
| 25 | 27 | ) | |
| 26 | 28 | ||
| 29 | + | // ErrUpdateSkipped indicates a filter update was intentionally skipped after a recoverable | |
| 30 | + | // update-path failure (e.g. invalid payload, merge failure, compile failure). | |
| 31 | + | var ErrUpdateSkipped = errors.New("filter update skipped") | |
| 32 | + | ||
| 27 | 33 | // Manager orchestrates the content filter lifecycle. | |
| 28 | 34 | // It handles downloading, compiling, loading, and applying filters to WebViews. | |
| 29 | 35 | type Manager struct { | |
| @@ -388,24 +394,24 @@ func (m *Manager) CheckForUpdates(ctx context.Context) error { | |||
| 388 | 394 | validateErr := m.validateDownloadedFiles(ctx, paths) | |
| 389 | 395 | if validateErr != nil { | |
| 390 | 396 | log.Warn().Err(validateErr).Msg("invalid downloaded filters, keeping existing active filter") | |
| 391 | - | return nil | |
| 397 | + | return fmt.Errorf("%w: validation failed: %w", ErrUpdateSkipped, validateErr) | |
| 392 | 398 | } | |
| 393 | 399 | ||
| 394 | 400 | // Merge and compile | |
| 395 | 401 | mergedPath, err := m.mergeJSONFiles(ctx, paths) | |
| 396 | 402 | if err != nil { | |
| 397 | 403 | log.Warn().Err(err).Msg("failed to merge updated filters, keeping existing active filter") | |
| 398 | - | return nil | |
| 404 | + | return fmt.Errorf("%w: merge failed: %w", ErrUpdateSkipped, err) | |
| 399 | 405 | } | |
| 400 | 406 | ||
| 401 | 407 | filter, err := m.store.Compile(ctx, FilterIdentifier, mergedPath) | |
| 402 | 408 | if err != nil { | |
| 403 | 409 | log.Warn().Err(err).Msg("failed to compile updated filters, keeping existing active filter") | |
| 404 | - | return nil | |
| 410 | + | return fmt.Errorf("%w: compile failed: %w", ErrUpdateSkipped, err) | |
| 405 | 411 | } | |
| 406 | 412 | if filter == nil { | |
| 407 | 413 | log.Warn().Msg("updated filter compilation returned nil filter, keeping existing active filter") | |
| 408 | - | return nil | |
| 414 | + | return fmt.Errorf("%w: compile returned nil filter", ErrUpdateSkipped) | |
| 409 | 415 | } | |
| 410 | 416 | ||
| 411 | 417 | m.filterMu.Lock() | |
| @@ -430,15 +436,35 @@ func (m *Manager) hasActiveFilter() bool { | |||
| 430 | 436 | } | |
| 431 | 437 | ||
| 432 | 438 | func (m *Manager) validateDownloadedFiles(ctx context.Context, paths []string) error { | |
| 439 | + | var validationErrs []error | |
| 440 | + | var quarantineErrs []error | |
| 441 | + | ||
| 433 | 442 | for _, path := range paths { | |
| 434 | 443 | if err := validateRuleFile(path); err != nil { | |
| 435 | 444 | if quarantineErr := m.quarantineInvalidFile(ctx, path, "rule_validation_failed"); quarantineErr != nil { | |
| 436 | - | return fmt.Errorf("invalid filter file %s: %w (quarantine failed: %s)", path, err, quarantineErr.Error()) | |
| 445 | + | quarantineErrs = append(quarantineErrs, fmt.Errorf("file=%s: %w", path, quarantineErr)) | |
| 437 | 446 | } | |
| 438 | - | return fmt.Errorf("invalid filter file %s: %w", path, err) | |
| 447 | + | validationErrs = append(validationErrs, fmt.Errorf("file=%s: %w", path, err)) | |
| 439 | 448 | } | |
| 440 | 449 | } | |
| 441 | - | return nil | |
| 450 | + | ||
| 451 | + | if len(validationErrs) == 0 && len(quarantineErrs) == 0 { | |
| 452 | + | return nil | |
| 453 | + | } | |
| 454 | + | ||
| 455 | + | var parts []string | |
| 456 | + | if len(validationErrs) > 0 { | |
| 457 | + | parts = append(parts, fmt.Sprintf("%d invalid file(s)", len(validationErrs))) | |
| 458 | + | } | |
| 459 | + | if len(quarantineErrs) > 0 { | |
| 460 | + | parts = append(parts, fmt.Sprintf("%d quarantine failure(s)", len(quarantineErrs))) | |
| 461 | + | } | |
| 462 | + | ||
| 463 | + | return fmt.Errorf( | |
| 464 | + | "downloaded filter validation failed (%s): %w", | |
| 465 | + | strings.Join(parts, ", "), | |
| 466 | + | errors.Join(append(validationErrs, quarantineErrs...)...), | |
| 467 | + | ) | |
| 442 | 468 | } | |
| 443 | 469 | ||
| 444 | 470 | func validateRuleFile(path string) error { | |
| @@ -3,9 +3,11 @@ package filtering | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "encoding/json" | 5 | "encoding/json" | ||
| 6 | + | "errors" | |||
| 6 | "fmt" | 7 | "fmt" | ||
| 7 | "os" | 8 | "os" | ||
| 8 | "path/filepath" | 9 | "path/filepath" | ||
| 10 | + | "strings" | |||
| 9 | "sync" | 11 | "sync" | ||
| 10 | "sync/atomic" | 12 | "sync/atomic" | ||
| 11 | "time" | 13 | "time" | ||
| @@ -24,6 +26,10 @@ const ( | |||||
| 24 | CacheMaxAge = 24 * time.Hour | 26 | CacheMaxAge = 24 * time.Hour | ||
| 25 | ) | 27 | ) | ||
| 26 | 28 | ||||
| 29 | + | // ErrUpdateSkipped indicates a filter update was intentionally skipped after a recoverable | |||
| 30 | + | // update-path failure (e.g. invalid payload, merge failure, compile failure). | |||
| 31 | + | var ErrUpdateSkipped = errors.New("filter update skipped") | |||
| 32 | + | ||||
| 27 | // Manager orchestrates the content filter lifecycle. | 33 | // Manager orchestrates the content filter lifecycle. | ||
| 28 | // It handles downloading, compiling, loading, and applying filters to WebViews. | 34 | // It handles downloading, compiling, loading, and applying filters to WebViews. | ||
| 29 | type Manager struct { | 35 | type Manager struct { | ||
| @@ -388,24 +394,24 @@ func (m *Manager) CheckForUpdates(ctx context.Context) error { | |||||
| 388 | validateErr := m.validateDownloadedFiles(ctx, paths) | 394 | validateErr := m.validateDownloadedFiles(ctx, paths) | ||
| 389 | if validateErr != nil { | 395 | if validateErr != nil { | ||
| 390 | log.Warn().Err(validateErr).Msg("invalid downloaded filters, keeping existing active filter") | 396 | log.Warn().Err(validateErr).Msg("invalid downloaded filters, keeping existing active filter") | ||
| 391 | - | return nil | 397 | + | return fmt.Errorf("%w: validation failed: %w", ErrUpdateSkipped, validateErr) |
| 392 | } | 398 | } | ||
| 393 | 399 | ||||
| 394 | // Merge and compile | 400 | // Merge and compile | ||
| 395 | mergedPath, err := m.mergeJSONFiles(ctx, paths) | 401 | mergedPath, err := m.mergeJSONFiles(ctx, paths) | ||
| 396 | if err != nil { | 402 | if err != nil { | ||
| 397 | log.Warn().Err(err).Msg("failed to merge updated filters, keeping existing active filter") | 403 | log.Warn().Err(err).Msg("failed to merge updated filters, keeping existing active filter") | ||
| 398 | - | return nil | 404 | + | return fmt.Errorf("%w: merge failed: %w", ErrUpdateSkipped, err) |
| 399 | } | 405 | } | ||
| 400 | 406 | ||||
| 401 | filter, err := m.store.Compile(ctx, FilterIdentifier, mergedPath) | 407 | filter, err := m.store.Compile(ctx, FilterIdentifier, mergedPath) | ||
| 402 | if err != nil { | 408 | if err != nil { | ||
| 403 | log.Warn().Err(err).Msg("failed to compile updated filters, keeping existing active filter") | 409 | log.Warn().Err(err).Msg("failed to compile updated filters, keeping existing active filter") | ||
| 404 | - | return nil | 410 | + | return fmt.Errorf("%w: compile failed: %w", ErrUpdateSkipped, err) |
| 405 | } | 411 | } | ||
| 406 | if filter == nil { | 412 | if filter == nil { | ||
| 407 | log.Warn().Msg("updated filter compilation returned nil filter, keeping existing active filter") | 413 | log.Warn().Msg("updated filter compilation returned nil filter, keeping existing active filter") | ||
| 408 | - | return nil | 414 | + | return fmt.Errorf("%w: compile returned nil filter", ErrUpdateSkipped) |
| 409 | } | 415 | } | ||
| 410 | 416 | ||||
| 411 | m.filterMu.Lock() | 417 | m.filterMu.Lock() | ||
| @@ -430,15 +436,35 @@ func (m *Manager) hasActiveFilter() bool { | |||||
| 430 | } | 436 | } | ||
| 431 | 437 | ||||
| 432 | func (m *Manager) validateDownloadedFiles(ctx context.Context, paths []string) error { | 438 | func (m *Manager) validateDownloadedFiles(ctx context.Context, paths []string) error { | ||
| 439 | + | var validationErrs []error | |||
| 440 | + | var quarantineErrs []error | |||
| 441 | + | ||||
| 433 | for _, path := range paths { | 442 | for _, path := range paths { | ||
| 434 | if err := validateRuleFile(path); err != nil { | 443 | if err := validateRuleFile(path); err != nil { | ||
| 435 | if quarantineErr := m.quarantineInvalidFile(ctx, path, "rule_validation_failed"); quarantineErr != nil { | 444 | if quarantineErr := m.quarantineInvalidFile(ctx, path, "rule_validation_failed"); quarantineErr != nil { | ||
| 436 | - | return fmt.Errorf("invalid filter file %s: %w (quarantine failed: %s)", path, err, quarantineErr.Error()) | 445 | + | quarantineErrs = append(quarantineErrs, fmt.Errorf("file=%s: %w", path, quarantineErr)) |
| 437 | } | 446 | } | ||
| 438 | - | return fmt.Errorf("invalid filter file %s: %w", path, err) | 447 | + | validationErrs = append(validationErrs, fmt.Errorf("file=%s: %w", path, err)) |
| 439 | } | 448 | } | ||
| 440 | } | 449 | } | ||
| 441 | - | return nil | 450 | + | |
| 451 | + | if len(validationErrs) == 0 && len(quarantineErrs) == 0 { | |||
| 452 | + | return nil | |||
| 453 | + | } | |||
| 454 | + | ||||
| 455 | + | var parts []string | |||
| 456 | + | if len(validationErrs) > 0 { | |||
| 457 | + | parts = append(parts, fmt.Sprintf("%d invalid file(s)", len(validationErrs))) | |||
| 458 | + | } | |||
| 459 | + | if len(quarantineErrs) > 0 { | |||
| 460 | + | parts = append(parts, fmt.Sprintf("%d quarantine failure(s)", len(quarantineErrs))) | |||
| 461 | + | } | |||
| 462 | + | ||||
| 463 | + | return fmt.Errorf( | |||
| 464 | + | "downloaded filter validation failed (%s): %w", | |||
| 465 | + | strings.Join(parts, ", "), | |||
| 466 | + | errors.Join(append(validationErrs, quarantineErrs...)...), | |||
| 467 | + | ) | |||
| 442 | } | 468 | } | ||
| 443 | 469 | ||||
| 444 | func validateRuleFile(path string) error { | 470 | func validateRuleFile(path string) error { | ||
M internal/infrastructure/filtering/manager_test.go +2 -2
| @@ -599,11 +599,11 @@ func TestManager_CheckForUpdates_CompileFailureKeepsActiveFilter(t *testing.T) { | |||
| 599 | 599 | mgr.LoadAsync(ctx) | |
| 600 | 600 | ||
| 601 | 601 | require.Eventually(t, func() bool { | |
| 602 | - | return mgr.GetFilter() != nil | |
| 602 | + | return mgr.GetFilter() != nil && mgr.Status().State == filtering.StateActive | |
| 603 | 603 | }, 3*time.Second, 50*time.Millisecond) | |
| 604 | 604 | ||
| 605 | 605 | err = mgr.CheckForUpdates(ctx) | |
| 606 | - | require.NoError(t, err) | |
| 606 | + | require.ErrorIs(t, err, filtering.ErrUpdateSkipped) | |
| 607 | 607 | assert.Same(t, activeFilter, mgr.GetFilter()) | |
| 608 | 608 | assert.Equal(t, filtering.StateActive, mgr.Status().State) | |
| 609 | 609 | } | |
| @@ -599,11 +599,11 @@ func TestManager_CheckForUpdates_CompileFailureKeepsActiveFilter(t *testing.T) { | |||||
| 599 | mgr.LoadAsync(ctx) | 599 | mgr.LoadAsync(ctx) | ||
| 600 | 600 | ||||
| 601 | require.Eventually(t, func() bool { | 601 | require.Eventually(t, func() bool { | ||
| 602 | - | return mgr.GetFilter() != nil | 602 | + | return mgr.GetFilter() != nil && mgr.Status().State == filtering.StateActive |
| 603 | }, 3*time.Second, 50*time.Millisecond) | 603 | }, 3*time.Second, 50*time.Millisecond) | ||
| 604 | 604 | ||||
| 605 | err = mgr.CheckForUpdates(ctx) | 605 | err = mgr.CheckForUpdates(ctx) | ||
| 606 | - | require.NoError(t, err) | 606 | + | require.ErrorIs(t, err, filtering.ErrUpdateSkipped) |
| 607 | assert.Same(t, activeFilter, mgr.GetFilter()) | 607 | assert.Same(t, activeFilter, mgr.GetFilter()) | ||
| 608 | assert.Equal(t, filtering.StateActive, mgr.Status().State) | 608 | assert.Equal(t, filtering.StateActive, mgr.Status().State) | ||
| 609 | } | 609 | } | ||
M internal/infrastructure/persistence/sqlite/session_state_repo.go +8 -6
| @@ -49,6 +49,11 @@ func (r *sessionStateRepo) SaveSnapshot(ctx context.Context, state *entity.Sessi | |||
| 49 | 49 | if err != nil { | |
| 50 | 50 | return fmt.Errorf("begin snapshot transaction: %w", err) | |
| 51 | 51 | } | |
| 52 | + | defer func() { | |
| 53 | + | if rollbackErr := tx.Rollback(); rollbackErr != nil && !errors.Is(rollbackErr, sql.ErrTxDone) { | |
| 54 | + | log.Debug().Err(rollbackErr).Msg("snapshot rollback reported non-terminal error") | |
| 55 | + | } | |
| 56 | + | }() | |
| 52 | 57 | ||
| 53 | 58 | txQueries := r.queries.WithTx(tx) | |
| 54 | 59 | if err := txQueries.UpsertSessionState(ctx, sqlc.UpsertSessionStateParams{ | |
| @@ -58,12 +63,9 @@ func (r *sessionStateRepo) SaveSnapshot(ctx context.Context, state *entity.Sessi | |||
| 58 | 63 | TabCount: int64(len(state.Tabs)), | |
| 59 | 64 | PaneCount: int64(state.CountPanes()), | |
| 60 | 65 | UpdatedAt: state.SavedAt, | |
| 61 | - | }); err != nil { | |
| 62 | - | if rollbackErr := tx.Rollback(); rollbackErr != nil && !errors.Is(rollbackErr, sql.ErrTxDone) { | |
| 63 | - | return fmt.Errorf("save snapshot rollback after upsert failure: %w", errors.Join(err, rollbackErr)) | |
| 64 | - | } | |
| 65 | - | return err | |
| 66 | - | } | |
| 66 | + | }); err != nil { | |
| 67 | + | return err | |
| 68 | + | } | |
| 67 | 69 | ||
| 68 | 70 | if err := tx.Commit(); err != nil { | |
| 69 | 71 | return fmt.Errorf("commit snapshot transaction: %w", err) | |
| @@ -49,6 +49,11 @@ func (r *sessionStateRepo) SaveSnapshot(ctx context.Context, state *entity.Sessi | |||||
| 49 | if err != nil { | 49 | if err != nil { | ||
| 50 | return fmt.Errorf("begin snapshot transaction: %w", err) | 50 | return fmt.Errorf("begin snapshot transaction: %w", err) | ||
| 51 | } | 51 | } | ||
| 52 | + | defer func() { | |||
| 53 | + | if rollbackErr := tx.Rollback(); rollbackErr != nil && !errors.Is(rollbackErr, sql.ErrTxDone) { | |||
| 54 | + | log.Debug().Err(rollbackErr).Msg("snapshot rollback reported non-terminal error") | |||
| 55 | + | } | |||
| 56 | + | }() | |||
| 52 | 57 | ||||
| 53 | txQueries := r.queries.WithTx(tx) | 58 | txQueries := r.queries.WithTx(tx) | ||
| 54 | if err := txQueries.UpsertSessionState(ctx, sqlc.UpsertSessionStateParams{ | 59 | if err := txQueries.UpsertSessionState(ctx, sqlc.UpsertSessionStateParams{ | ||
| @@ -58,12 +63,9 @@ func (r *sessionStateRepo) SaveSnapshot(ctx context.Context, state *entity.Sessi | |||||
| 58 | TabCount: int64(len(state.Tabs)), | 63 | TabCount: int64(len(state.Tabs)), | ||
| 59 | PaneCount: int64(state.CountPanes()), | 64 | PaneCount: int64(state.CountPanes()), | ||
| 60 | UpdatedAt: state.SavedAt, | 65 | UpdatedAt: state.SavedAt, | ||
| 61 | - | }); err != nil { | 66 | + | }); err != nil { |
| 62 | - | if rollbackErr := tx.Rollback(); rollbackErr != nil && !errors.Is(rollbackErr, sql.ErrTxDone) { | 67 | + | return err |
| 63 | - | return fmt.Errorf("save snapshot rollback after upsert failure: %w", errors.Join(err, rollbackErr)) | 68 | + | } |
| 64 | - | } | |||
| 65 | - | return err | |||
| 66 | - | } | |||
| 67 | 69 | ||||
| 68 | if err := tx.Commit(); err != nil { | 70 | if err := tx.Commit(); err != nil { | ||
| 69 | return fmt.Errorf("commit snapshot transaction: %w", err) | 71 | return fmt.Errorf("commit snapshot transaction: %w", err) | ||
M internal/infrastructure/snapshot/service.go +4 -0
| @@ -236,6 +236,10 @@ func waitForRetry(ctx context.Context, delay time.Duration) error { | |||
| 236 | 236 | } | |
| 237 | 237 | ||
| 238 | 238 | func isTransientFKError(err error) bool { | |
| 239 | + | // SQLite-specific transient FK detection: | |
| 240 | + | // we match case-insensitive "foreign key" text from wrapped driver errors. | |
| 241 | + | // Message wording may vary across SQLite versions/wrappers; adjust this check | |
| 242 | + | // if transient FK retries stop matching expected failures in production logs. | |
| 239 | 243 | if err == nil { | |
| 240 | 244 | return false | |
| 241 | 245 | } | |
| @@ -236,6 +236,10 @@ func waitForRetry(ctx context.Context, delay time.Duration) error { | |||||
| 236 | } | 236 | } | ||
| 237 | 237 | ||||
| 238 | func isTransientFKError(err error) bool { | 238 | func isTransientFKError(err error) bool { | ||
| 239 | + | // SQLite-specific transient FK detection: | |||
| 240 | + | // we match case-insensitive "foreign key" text from wrapped driver errors. | |||
| 241 | + | // Message wording may vary across SQLite versions/wrappers; adjust this check | |||
| 242 | + | // if transient FK retries stop matching expected failures in production logs. | |||
| 239 | if err == nil { | 243 | if err == nil { | ||
| 240 | return false | 244 | return false | ||
| 241 | } | 245 | } | ||
M internal/infrastructure/snapshot/service_test.go +4 -1
| @@ -134,5 +134,8 @@ func TestService_SetReady_SavesPendingDirtySnapshot(t *testing.T) { | |||
| 134 | 134 | case <-time.After(500 * time.Millisecond): | |
| 135 | 135 | t.Fatal("expected pending snapshot to be saved after SetReady") | |
| 136 | 136 | } | |
| 137 | - | assert.False(t, svc.dirty) | |
| 137 | + | svc.mu.Lock() | |
| 138 | + | dirty := svc.dirty | |
| 139 | + | svc.mu.Unlock() | |
| 140 | + | assert.False(t, dirty) | |
| 138 | 141 | } | |
| @@ -134,5 +134,8 @@ func TestService_SetReady_SavesPendingDirtySnapshot(t *testing.T) { | |||||
| 134 | case <-time.After(500 * time.Millisecond): | 134 | case <-time.After(500 * time.Millisecond): | ||
| 135 | t.Fatal("expected pending snapshot to be saved after SetReady") | 135 | t.Fatal("expected pending snapshot to be saved after SetReady") | ||
| 136 | } | 136 | } | ||
| 137 | - | assert.False(t, svc.dirty) | 137 | + | svc.mu.Lock() |
| 138 | + | dirty := svc.dirty | |||
| 139 | + | svc.mu.Unlock() | |||
| 140 | + | assert.False(t, dirty) | |||
| 138 | } | 141 | } | ||
M internal/infrastructure/updater/github.go +16 -28
| @@ -305,20 +305,26 @@ func retryDelayForAttempt(attempt int, randInt63 func(n int64) int64) time.Durat | |||
| 305 | 305 | } | |
| 306 | 306 | ||
| 307 | 307 | if randInt63 != nil && retryJitterMax > 0 { | |
| 308 | - | delay += time.Duration(randInt63(int64(retryJitterMax) + 1)) | |
| 308 | + | delay += time.Duration(randInt63(int64(retryJitterMax))) | |
| 309 | 309 | } | |
| 310 | 310 | ||
| 311 | 311 | return delay | |
| 312 | 312 | } | |
| 313 | 313 | ||
| 314 | - | func (g *GitHubChecker) doRequestWithRetry(ctx context.Context, req *http.Request) (*http.Response, error) { | |
| 314 | + | func doRequestWithRetryHelper( | |
| 315 | + | ctx context.Context, | |
| 316 | + | client *http.Client, | |
| 317 | + | req *http.Request, | |
| 318 | + | sleep func(ctx context.Context, d time.Duration) error, | |
| 319 | + | randInt63 func(n int64) int64, | |
| 320 | + | ) (*http.Response, error) { | |
| 315 | 321 | for attempt := 1; attempt <= maxRetryAttempts; attempt++ { | |
| 316 | - | resp, err := g.client.Do(req) | |
| 322 | + | resp, err := client.Do(req) | |
| 317 | 323 | if err != nil { | |
| 318 | 324 | if !isRetryableRequestError(err) || attempt == maxRetryAttempts { | |
| 319 | 325 | return nil, err | |
| 320 | 326 | } | |
| 321 | - | if waitErr := g.sleep(ctx, retryDelayForAttempt(attempt, g.randInt63)); waitErr != nil { | |
| 327 | + | if waitErr := sleep(ctx, retryDelayForAttempt(attempt, randInt63)); waitErr != nil { | |
| 322 | 328 | return nil, waitErr | |
| 323 | 329 | } | |
| 324 | 330 | continue | |
| @@ -329,7 +335,7 @@ func (g *GitHubChecker) doRequestWithRetry(ctx context.Context, req *http.Reques | |||
| 329 | 335 | } | |
| 330 | 336 | ||
| 331 | 337 | _ = resp.Body.Close() | |
| 332 | - | if waitErr := g.sleep(ctx, retryDelayForAttempt(attempt, g.randInt63)); waitErr != nil { | |
| 338 | + | if waitErr := sleep(ctx, retryDelayForAttempt(attempt, randInt63)); waitErr != nil { | |
| 333 | 339 | return nil, waitErr | |
| 334 | 340 | } | |
| 335 | 341 | } | |
| @@ -337,6 +343,10 @@ func (g *GitHubChecker) doRequestWithRetry(ctx context.Context, req *http.Reques | |||
| 337 | 343 | return nil, fmt.Errorf("request failed after retries") | |
| 338 | 344 | } | |
| 339 | 345 | ||
| 346 | + | func (g *GitHubChecker) doRequestWithRetry(ctx context.Context, req *http.Request) (*http.Response, error) { | |
| 347 | + | return doRequestWithRetryHelper(ctx, g.client, req, g.sleep, g.randInt63) | |
| 348 | + | } | |
| 349 | + | ||
| 340 | 350 | // validateDownloadURL ensures the URL is a valid GitHub releases URL. | |
| 341 | 351 | func validateDownloadURL(downloadURL string) error { | |
| 342 | 352 | parsed, err := url.Parse(downloadURL) | |
| @@ -537,29 +547,7 @@ func (g *GitHubDownloader) Download(ctx context.Context, downloadURL, destDir st | |||
| 537 | 547 | } | |
| 538 | 548 | ||
| 539 | 549 | func (g *GitHubDownloader) doRequestWithRetry(ctx context.Context, req *http.Request) (*http.Response, error) { | |
| 540 | - | for attempt := 1; attempt <= maxRetryAttempts; attempt++ { | |
| 541 | - | resp, err := g.client.Do(req) | |
| 542 | - | if err != nil { | |
| 543 | - | if !isRetryableRequestError(err) || attempt == maxRetryAttempts { | |
| 544 | - | return nil, err | |
| 545 | - | } | |
| 546 | - | if waitErr := g.sleep(ctx, retryDelayForAttempt(attempt, g.randInt63)); waitErr != nil { | |
| 547 | - | return nil, waitErr | |
| 548 | - | } | |
| 549 | - | continue | |
| 550 | - | } | |
| 551 | - | ||
| 552 | - | if !isRetryableStatus(resp.StatusCode) || attempt == maxRetryAttempts { | |
| 553 | - | return resp, nil | |
| 554 | - | } | |
| 555 | - | ||
| 556 | - | _ = resp.Body.Close() | |
| 557 | - | if waitErr := g.sleep(ctx, retryDelayForAttempt(attempt, g.randInt63)); waitErr != nil { | |
| 558 | - | return nil, waitErr | |
| 559 | - | } | |
| 560 | - | } | |
| 561 | - | ||
| 562 | - | return nil, fmt.Errorf("request failed after retries") | |
| 550 | + | return doRequestWithRetryHelper(ctx, g.client, req, g.sleep, g.randInt63) | |
| 563 | 551 | } | |
| 564 | 552 | ||
| 565 | 553 | // sanitizeTarPath validates and sanitizes a tar header name to prevent path traversal. | |
| @@ -305,20 +305,26 @@ func retryDelayForAttempt(attempt int, randInt63 func(n int64) int64) time.Durat | |||||
| 305 | } | 305 | } | ||
| 306 | 306 | ||||
| 307 | if randInt63 != nil && retryJitterMax > 0 { | 307 | if randInt63 != nil && retryJitterMax > 0 { | ||
| 308 | - | delay += time.Duration(randInt63(int64(retryJitterMax) + 1)) | 308 | + | delay += time.Duration(randInt63(int64(retryJitterMax))) |
| 309 | } | 309 | } | ||
| 310 | 310 | ||||
| 311 | return delay | 311 | return delay | ||
| 312 | } | 312 | } | ||
| 313 | 313 | ||||
| 314 | - | func (g *GitHubChecker) doRequestWithRetry(ctx context.Context, req *http.Request) (*http.Response, error) { | 314 | + | func doRequestWithRetryHelper( |
| 315 | + | ctx context.Context, | |||
| 316 | + | client *http.Client, | |||
| 317 | + | req *http.Request, | |||
| 318 | + | sleep func(ctx context.Context, d time.Duration) error, | |||
| 319 | + | randInt63 func(n int64) int64, | |||
| 320 | + | ) (*http.Response, error) { | |||
| 315 | for attempt := 1; attempt <= maxRetryAttempts; attempt++ { | 321 | for attempt := 1; attempt <= maxRetryAttempts; attempt++ { | ||
| 316 | - | resp, err := g.client.Do(req) | 322 | + | resp, err := client.Do(req) |
| 317 | if err != nil { | 323 | if err != nil { | ||
| 318 | if !isRetryableRequestError(err) || attempt == maxRetryAttempts { | 324 | if !isRetryableRequestError(err) || attempt == maxRetryAttempts { | ||
| 319 | return nil, err | 325 | return nil, err | ||
| 320 | } | 326 | } | ||
| 321 | - | if waitErr := g.sleep(ctx, retryDelayForAttempt(attempt, g.randInt63)); waitErr != nil { | 327 | + | if waitErr := sleep(ctx, retryDelayForAttempt(attempt, randInt63)); waitErr != nil { |
| 322 | return nil, waitErr | 328 | return nil, waitErr | ||
| 323 | } | 329 | } | ||
| 324 | continue | 330 | continue | ||
| @@ -329,7 +335,7 @@ func (g *GitHubChecker) doRequestWithRetry(ctx context.Context, req *http.Reques | |||||
| 329 | } | 335 | } | ||
| 330 | 336 | ||||
| 331 | _ = resp.Body.Close() | 337 | _ = resp.Body.Close() | ||
| 332 | - | if waitErr := g.sleep(ctx, retryDelayForAttempt(attempt, g.randInt63)); waitErr != nil { | 338 | + | if waitErr := sleep(ctx, retryDelayForAttempt(attempt, randInt63)); waitErr != nil { |
| 333 | return nil, waitErr | 339 | return nil, waitErr | ||
| 334 | } | 340 | } | ||
| 335 | } | 341 | } | ||
| @@ -337,6 +343,10 @@ func (g *GitHubChecker) doRequestWithRetry(ctx context.Context, req *http.Reques | |||||
| 337 | return nil, fmt.Errorf("request failed after retries") | 343 | return nil, fmt.Errorf("request failed after retries") | ||
| 338 | } | 344 | } | ||
| 339 | 345 | ||||
| 346 | + | func (g *GitHubChecker) doRequestWithRetry(ctx context.Context, req *http.Request) (*http.Response, error) { | |||
| 347 | + | return doRequestWithRetryHelper(ctx, g.client, req, g.sleep, g.randInt63) | |||
| 348 | + | } | |||
| 349 | + | ||||
| 340 | // validateDownloadURL ensures the URL is a valid GitHub releases URL. | 350 | // validateDownloadURL ensures the URL is a valid GitHub releases URL. | ||
| 341 | func validateDownloadURL(downloadURL string) error { | 351 | func validateDownloadURL(downloadURL string) error { | ||
| 342 | parsed, err := url.Parse(downloadURL) | 352 | parsed, err := url.Parse(downloadURL) | ||
| @@ -537,29 +547,7 @@ func (g *GitHubDownloader) Download(ctx context.Context, downloadURL, destDir st | |||||
| 537 | } | 547 | } | ||
| 538 | 548 | ||||
| 539 | func (g *GitHubDownloader) doRequestWithRetry(ctx context.Context, req *http.Request) (*http.Response, error) { | 549 | func (g *GitHubDownloader) doRequestWithRetry(ctx context.Context, req *http.Request) (*http.Response, error) { | ||
| 540 | - | for attempt := 1; attempt <= maxRetryAttempts; attempt++ { | 550 | + | return doRequestWithRetryHelper(ctx, g.client, req, g.sleep, g.randInt63) |
| 541 | - | resp, err := g.client.Do(req) | |||
| 542 | - | if err != nil { | |||
| 543 | - | if !isRetryableRequestError(err) || attempt == maxRetryAttempts { | |||
| 544 | - | return nil, err | |||
| 545 | - | } | |||
| 546 | - | if waitErr := g.sleep(ctx, retryDelayForAttempt(attempt, g.randInt63)); waitErr != nil { | |||
| 547 | - | return nil, waitErr | |||
| 548 | - | } | |||
| 549 | - | continue | |||
| 550 | - | } | |||
| 551 | - | ||||
| 552 | - | if !isRetryableStatus(resp.StatusCode) || attempt == maxRetryAttempts { | |||
| 553 | - | return resp, nil | |||
| 554 | - | } | |||
| 555 | - | ||||
| 556 | - | _ = resp.Body.Close() | |||
| 557 | - | if waitErr := g.sleep(ctx, retryDelayForAttempt(attempt, g.randInt63)); waitErr != nil { | |||
| 558 | - | return nil, waitErr | |||
| 559 | - | } | |||
| 560 | - | } | |||
| 561 | - | ||||
| 562 | - | return nil, fmt.Errorf("request failed after retries") | |||
| 563 | } | 551 | } | ||
| 564 | 552 | ||||
| 565 | // sanitizeTarPath validates and sanitizes a tar header name to prevent path traversal. | 553 | // sanitizeTarPath validates and sanitizes a tar header name to prevent path traversal. | ||
M internal/infrastructure/webkit/scheme_handler.go +11 -2
| @@ -169,7 +169,7 @@ func (h *DumbSchemeHandler) registerDefaults() { | |||
| 169 | 169 | if req.Method != "" && req.Method != httpGET { | |
| 170 | 170 | return nil | |
| 171 | 171 | } | |
| 172 | - | originalURI := crashOriginalURI(req.URI) | |
| 172 | + | originalURI := sanitizeCrashPageOriginalURI(crashOriginalURI(req.URI)) | |
| 173 | 173 | return &SchemeResponse{ | |
| 174 | 174 | Data: []byte(buildCrashPageHTML(originalURI)), | |
| 175 | 175 | ContentType: "text/html; charset=utf-8", | |
| @@ -207,7 +207,7 @@ func crashOriginalURI(requestURI string) string { | |||
| 207 | 207 | return strings.TrimSpace(parsed.Query().Get("url")) | |
| 208 | 208 | } | |
| 209 | 209 | ||
| 210 | - | func crashReloadTarget(originalURI string) string { | |
| 210 | + | func sanitizeCrashPageOriginalURI(originalURI string) string { | |
| 211 | 211 | originalURI = strings.TrimSpace(originalURI) | |
| 212 | 212 | if originalURI == "" { | |
| 213 | 213 | return "" | |
| @@ -222,11 +222,20 @@ func crashReloadTarget(originalURI string) string { | |||
| 222 | 222 | return "" | |
| 223 | 223 | } | |
| 224 | 224 | return parsed.String() | |
| 225 | + | case "dumb": | |
| 226 | + | if parsed.Host == "" && parsed.Opaque == "" { | |
| 227 | + | return "" | |
| 228 | + | } | |
| 229 | + | return parsed.String() | |
| 225 | 230 | default: | |
| 226 | 231 | return "" | |
| 227 | 232 | } | |
| 228 | 233 | } | |
| 229 | 234 | ||
| 235 | + | func crashReloadTarget(originalURI string) string { | |
| 236 | + | return sanitizeCrashPageOriginalURI(originalURI) | |
| 237 | + | } | |
| 238 | + | ||
| 230 | 239 | func buildCrashPageHTML(originalURI string) string { | |
| 231 | 240 | escapedOriginal := html.EscapeString(originalURI) | |
| 232 | 241 | escapedReloadTarget := html.EscapeString(crashReloadTarget(originalURI)) | |
| @@ -169,7 +169,7 @@ func (h *DumbSchemeHandler) registerDefaults() { | |||||
| 169 | if req.Method != "" && req.Method != httpGET { | 169 | if req.Method != "" && req.Method != httpGET { | ||
| 170 | return nil | 170 | return nil | ||
| 171 | } | 171 | } | ||
| 172 | - | originalURI := crashOriginalURI(req.URI) | 172 | + | originalURI := sanitizeCrashPageOriginalURI(crashOriginalURI(req.URI)) |
| 173 | return &SchemeResponse{ | 173 | return &SchemeResponse{ | ||
| 174 | Data: []byte(buildCrashPageHTML(originalURI)), | 174 | Data: []byte(buildCrashPageHTML(originalURI)), | ||
| 175 | ContentType: "text/html; charset=utf-8", | 175 | ContentType: "text/html; charset=utf-8", | ||
| @@ -207,7 +207,7 @@ func crashOriginalURI(requestURI string) string { | |||||
| 207 | return strings.TrimSpace(parsed.Query().Get("url")) | 207 | return strings.TrimSpace(parsed.Query().Get("url")) | ||
| 208 | } | 208 | } | ||
| 209 | 209 | ||||
| 210 | - | func crashReloadTarget(originalURI string) string { | 210 | + | func sanitizeCrashPageOriginalURI(originalURI string) string { |
| 211 | originalURI = strings.TrimSpace(originalURI) | 211 | originalURI = strings.TrimSpace(originalURI) | ||
| 212 | if originalURI == "" { | 212 | if originalURI == "" { | ||
| 213 | return "" | 213 | return "" | ||
| @@ -222,11 +222,20 @@ func crashReloadTarget(originalURI string) string { | |||||
| 222 | return "" | 222 | return "" | ||
| 223 | } | 223 | } | ||
| 224 | return parsed.String() | 224 | return parsed.String() | ||
| 225 | + | case "dumb": | |||
| 226 | + | if parsed.Host == "" && parsed.Opaque == "" { | |||
| 227 | + | return "" | |||
| 228 | + | } | |||
| 229 | + | return parsed.String() | |||
| 225 | default: | 230 | default: | ||
| 226 | return "" | 231 | return "" | ||
| 227 | } | 232 | } | ||
| 228 | } | 233 | } | ||
| 229 | 234 | ||||
| 235 | + | func crashReloadTarget(originalURI string) string { | |||
| 236 | + | return sanitizeCrashPageOriginalURI(originalURI) | |||
| 237 | + | } | |||
| 238 | + | ||||
| 230 | func buildCrashPageHTML(originalURI string) string { | 239 | func buildCrashPageHTML(originalURI string) string { | ||
| 231 | escapedOriginal := html.EscapeString(originalURI) | 240 | escapedOriginal := html.EscapeString(originalURI) | ||
| 232 | escapedReloadTarget := html.EscapeString(crashReloadTarget(originalURI)) | 241 | escapedReloadTarget := html.EscapeString(crashReloadTarget(originalURI)) | ||
M internal/infrastructure/webkit/scheme_handler_test.go +25 -0
| @@ -30,6 +30,11 @@ func TestBuildCrashPageHTMLRejectsUnsafeReloadScheme(t *testing.T) { | |||
| 30 | 30 | assert.Contains(t, body, `data-target=""`) | |
| 31 | 31 | } | |
| 32 | 32 | ||
| 33 | + | func TestBuildCrashPageHTMLAllowsDumbSchemeTarget(t *testing.T) { | |
| 34 | + | body := buildCrashPageHTML("dumb://home") | |
| 35 | + | assert.Contains(t, body, `data-target="dumb://home"`) | |
| 36 | + | } | |
| 37 | + | ||
| 33 | 38 | func TestBuildCrashPageHTMLEscapesScriptBreakoutPayload(t *testing.T) { | |
| 34 | 39 | payload := `https://example.com/?q=</script><script>alert(1)</script>` | |
| 35 | 40 | body := buildCrashPageHTML(payload) | |
| @@ -58,3 +63,23 @@ func TestRegisterDefaultsIncludesCrashHandler(t *testing.T) { | |||
| 58 | 63 | assert.Equal(t, "text/html; charset=utf-8", resp.ContentType) | |
| 59 | 64 | assert.Contains(t, string(resp.Data), "Renderer process ended") | |
| 60 | 65 | } | |
| 66 | + | ||
| 67 | + | func TestCrashHandlerSanitizesUnsafeURLQuery(t *testing.T) { | |
| 68 | + | handler := NewDumbSchemeHandler(context.Background()) | |
| 69 | + | require.NotNil(t, handler) | |
| 70 | + | ||
| 71 | + | handler.mu.RLock() | |
| 72 | + | crashHandler, ok := handler.handlers["/crash"] | |
| 73 | + | handler.mu.RUnlock() | |
| 74 | + | require.True(t, ok) | |
| 75 | + | require.NotNil(t, crashHandler) | |
| 76 | + | ||
| 77 | + | resp := crashHandler.Handle(&SchemeRequest{ | |
| 78 | + | URI: "dumb://home/crash?url=javascript%3Aalert(1)", | |
| 79 | + | Path: "/crash", | |
| 80 | + | Method: "GET", | |
| 81 | + | Scheme: "dumb", | |
| 82 | + | }) | |
| 83 | + | require.NotNil(t, resp) | |
| 84 | + | assert.Contains(t, string(resp.Data), `data-target=""`) | |
| 85 | + | } | |
| @@ -30,6 +30,11 @@ func TestBuildCrashPageHTMLRejectsUnsafeReloadScheme(t *testing.T) { | |||||
| 30 | assert.Contains(t, body, `data-target=""`) | 30 | assert.Contains(t, body, `data-target=""`) | ||
| 31 | } | 31 | } | ||
| 32 | 32 | ||||
| 33 | + | func TestBuildCrashPageHTMLAllowsDumbSchemeTarget(t *testing.T) { | |||
| 34 | + | body := buildCrashPageHTML("dumb://home") | |||
| 35 | + | assert.Contains(t, body, `data-target="dumb://home"`) | |||
| 36 | + | } | |||
| 37 | + | ||||
| 33 | func TestBuildCrashPageHTMLEscapesScriptBreakoutPayload(t *testing.T) { | 38 | func TestBuildCrashPageHTMLEscapesScriptBreakoutPayload(t *testing.T) { | ||
| 34 | payload := `https://example.com/?q=</script><script>alert(1)</script>` | 39 | payload := `https://example.com/?q=</script><script>alert(1)</script>` | ||
| 35 | body := buildCrashPageHTML(payload) | 40 | body := buildCrashPageHTML(payload) | ||
| @@ -58,3 +63,23 @@ func TestRegisterDefaultsIncludesCrashHandler(t *testing.T) { | |||||
| 58 | assert.Equal(t, "text/html; charset=utf-8", resp.ContentType) | 63 | assert.Equal(t, "text/html; charset=utf-8", resp.ContentType) | ||
| 59 | assert.Contains(t, string(resp.Data), "Renderer process ended") | 64 | assert.Contains(t, string(resp.Data), "Renderer process ended") | ||
| 60 | } | 65 | } | ||
| 66 | + | ||||
| 67 | + | func TestCrashHandlerSanitizesUnsafeURLQuery(t *testing.T) { | |||
| 68 | + | handler := NewDumbSchemeHandler(context.Background()) | |||
| 69 | + | require.NotNil(t, handler) | |||
| 70 | + | ||||
| 71 | + | handler.mu.RLock() | |||
| 72 | + | crashHandler, ok := handler.handlers["/crash"] | |||
| 73 | + | handler.mu.RUnlock() | |||
| 74 | + | require.True(t, ok) | |||
| 75 | + | require.NotNil(t, crashHandler) | |||
| 76 | + | ||||
| 77 | + | resp := crashHandler.Handle(&SchemeRequest{ | |||
| 78 | + | URI: "dumb://home/crash?url=javascript%3Aalert(1)", | |||
| 79 | + | Path: "/crash", | |||
| 80 | + | Method: "GET", | |||
| 81 | + | Scheme: "dumb", | |||
| 82 | + | }) | |||
| 83 | + | require.NotNil(t, resp) | |||
| 84 | + | assert.Contains(t, string(resp.Data), `data-target=""`) | |||
| 85 | + | } | |||
M internal/infrastructure/webkit/webview.go +2 -0
| @@ -1416,11 +1416,13 @@ func (wv *WebView) ResetForPoolReuse() { | |||
| 1416 | 1416 | wv.canGoFwd = false | |
| 1417 | 1417 | wv.isLoading = false | |
| 1418 | 1418 | wv.asyncCallbacks = nil | |
| 1419 | + | wv.runJSErrorStats = make(map[string]runJSErrorStat) | |
| 1419 | 1420 | wv.mu.Unlock() | |
| 1420 | 1421 | ||
| 1421 | 1422 | wv.isFullscreen.Store(false) | |
| 1422 | 1423 | wv.isPlayingAudio.Store(false) | |
| 1423 | 1424 | wv.navigationActive.Store(false) | |
| 1425 | + | wv.lastProgressUpdate.Store(0) | |
| 1424 | 1426 | ||
| 1425 | 1427 | if wv.inner != nil { | |
| 1426 | 1428 | wv.inner.StopLoading() | |
| @@ -1416,11 +1416,13 @@ func (wv *WebView) ResetForPoolReuse() { | |||||
| 1416 | wv.canGoFwd = false | 1416 | wv.canGoFwd = false | ||
| 1417 | wv.isLoading = false | 1417 | wv.isLoading = false | ||
| 1418 | wv.asyncCallbacks = nil | 1418 | wv.asyncCallbacks = nil | ||
| 1419 | + | wv.runJSErrorStats = make(map[string]runJSErrorStat) | |||
| 1419 | wv.mu.Unlock() | 1420 | wv.mu.Unlock() | ||
| 1420 | 1421 | ||||
| 1421 | wv.isFullscreen.Store(false) | 1422 | wv.isFullscreen.Store(false) | ||
| 1422 | wv.isPlayingAudio.Store(false) | 1423 | wv.isPlayingAudio.Store(false) | ||
| 1423 | wv.navigationActive.Store(false) | 1424 | wv.navigationActive.Store(false) | ||
| 1425 | + | wv.lastProgressUpdate.Store(0) | |||
| 1424 | 1426 | ||||
| 1425 | if wv.inner != nil { | 1427 | if wv.inner != nil { | ||
| 1426 | wv.inner.StopLoading() | 1428 | wv.inner.StopLoading() | ||
M internal/infrastructure/webkit/webview_termination_test.go +5 -5
| @@ -26,9 +26,9 @@ func TestResolveTerminatePolicy(t *testing.T) { | |||
| 26 | 26 | } | |
| 27 | 27 | ||
| 28 | 28 | func TestShouldTerminateWebProcess(t *testing.T) { | |
| 29 | - | assert.False(t, shouldTerminateWebProcess(terminatePolicyAlways, true, true)) | |
| 30 | - | assert.False(t, shouldTerminateWebProcess(terminatePolicyNever, false, true)) | |
| 31 | - | assert.True(t, shouldTerminateWebProcess(terminatePolicyAlways, false, false)) | |
| 32 | - | assert.True(t, shouldTerminateWebProcess(terminatePolicyAuto, false, true)) | |
| 33 | - | assert.False(t, shouldTerminateWebProcess(terminatePolicyAuto, false, false)) | |
| 29 | + | assert.False(t, shouldTerminateWebProcess(terminatePolicyAlways, true, true)) // isRelated=true, navigationActive=true | |
| 30 | + | assert.False(t, shouldTerminateWebProcess(terminatePolicyNever, false, true)) // isRelated=false, navigationActive=true | |
| 31 | + | assert.True(t, shouldTerminateWebProcess(terminatePolicyAlways, false, false)) // isRelated=false, navigationActive=false | |
| 32 | + | assert.True(t, shouldTerminateWebProcess(terminatePolicyAuto, false, true)) // isRelated=false, navigationActive=true | |
| 33 | + | assert.False(t, shouldTerminateWebProcess(terminatePolicyAuto, false, false)) // isRelated=false, navigationActive=false | |
| 34 | 34 | } | |
| @@ -26,9 +26,9 @@ func TestResolveTerminatePolicy(t *testing.T) { | |||||
| 26 | } | 26 | } | ||
| 27 | 27 | ||||
| 28 | func TestShouldTerminateWebProcess(t *testing.T) { | 28 | func TestShouldTerminateWebProcess(t *testing.T) { | ||
| 29 | - | assert.False(t, shouldTerminateWebProcess(terminatePolicyAlways, true, true)) | 29 | + | assert.False(t, shouldTerminateWebProcess(terminatePolicyAlways, true, true)) // isRelated=true, navigationActive=true |
| 30 | - | assert.False(t, shouldTerminateWebProcess(terminatePolicyNever, false, true)) | 30 | + | assert.False(t, shouldTerminateWebProcess(terminatePolicyNever, false, true)) // isRelated=false, navigationActive=true |
| 31 | - | assert.True(t, shouldTerminateWebProcess(terminatePolicyAlways, false, false)) | 31 | + | assert.True(t, shouldTerminateWebProcess(terminatePolicyAlways, false, false)) // isRelated=false, navigationActive=false |
| 32 | - | assert.True(t, shouldTerminateWebProcess(terminatePolicyAuto, false, true)) | 32 | + | assert.True(t, shouldTerminateWebProcess(terminatePolicyAuto, false, true)) // isRelated=false, navigationActive=true |
| 33 | - | assert.False(t, shouldTerminateWebProcess(terminatePolicyAuto, false, false)) | 33 | + | assert.False(t, shouldTerminateWebProcess(terminatePolicyAuto, false, false)) // isRelated=false, navigationActive=false |
| 34 | } | 34 | } | ||
M internal/ui/adapter/favicon.go +12 -14
| @@ -26,6 +26,8 @@ type FaviconAdapter struct { | |||
| 26 | 26 | warnCounts map[string]int | |
| 27 | 27 | } | |
| 28 | 28 | ||
| 29 | + | type warningLogFunc func(log *zerolog.Logger, err error) | |
| 30 | + | ||
| 29 | 31 | // NewFaviconAdapter creates a new FaviconAdapter. | |
| 30 | 32 | // The faviconDB can be nil if WebKit favicon database is not available. | |
| 31 | 33 | func NewFaviconAdapter(service *favicon.Service, faviconDB *webkit.FaviconDatabase) *FaviconAdapter { | |
| @@ -210,8 +212,8 @@ func (a *FaviconAdapter) saveFaviconToDisk(ctx context.Context, domain string, t | |||
| 210 | 212 | if needsPNGSave { | |
| 211 | 213 | // Ensure cache directory exists (cheap check, do it now) | |
| 212 | 214 | if err := a.service.EnsureCacheDir(); err != nil { | |
| 213 | - | a.logWarningDedup(ctx, cacheDirWarnKey, err, func(e *zerolog.Event) { | |
| 214 | - | e.Str("domain", domain).Msg("failed to create favicon cache dir") | |
| 215 | + | a.logWarningDedup(ctx, cacheDirWarnKey, err, func(log *zerolog.Logger, warnErr error) { | |
| 216 | + | log.Warn().Err(warnErr).Str("domain", domain).Msg("failed to create favicon cache dir") | |
| 215 | 217 | }) | |
| 216 | 218 | needsPNGSave = false | |
| 217 | 219 | } else { | |
| @@ -224,8 +226,8 @@ func (a *FaviconAdapter) saveFaviconToDisk(ctx context.Context, domain string, t | |||
| 224 | 226 | // texture.SaveToPng must run on main thread but we defer it to avoid blocking | |
| 225 | 227 | cb := glib.SourceFunc(func(_ uintptr) bool { | |
| 226 | 228 | if ok := texture.SaveToPng(pngPath); !ok { | |
| 227 | - | a.logWarningDedup(ctx, savePNGWarnKey, nil, func(e *zerolog.Event) { | |
| 228 | - | e.Str("domain", domain).Str("path", pngPath).Msg("failed to save favicon PNG") | |
| 229 | + | a.logWarningDedup(ctx, savePNGWarnKey, nil, func(log *zerolog.Logger, _ error) { | |
| 230 | + | log.Warn().Str("domain", domain).Str("path", pngPath).Msg("failed to save favicon PNG") | |
| 229 | 231 | }) | |
| 230 | 232 | } else { | |
| 231 | 233 | a.clearWarningDedup(savePNGWarnKey) | |
| @@ -234,8 +236,8 @@ func (a *FaviconAdapter) saveFaviconToDisk(ctx context.Context, domain string, t | |||
| 234 | 236 | // Create normalized sized copy for dmenu/fuzzel (async) | |
| 235 | 237 | go func() { | |
| 236 | 238 | if err := a.service.EnsureSizedPNG(ctx, domain, favicon.NormalizedIconSize); err != nil { | |
| 237 | - | a.logWarningDedup(ctx, sizedPNGWarnKey, err, func(e *zerolog.Event) { | |
| 238 | - | e.Str("domain", domain).Msg("failed to create sized PNG") | |
| 239 | + | a.logWarningDedup(ctx, sizedPNGWarnKey, err, func(log *zerolog.Logger, warnErr error) { | |
| 240 | + | log.Warn().Err(warnErr).Str("domain", domain).Msg("failed to create sized PNG") | |
| 239 | 241 | }) | |
| 240 | 242 | } else { | |
| 241 | 243 | a.clearWarningDedup(sizedPNGWarnKey) | |
| @@ -335,8 +337,8 @@ func (a *FaviconAdapter) ensureSizedPNG(ctx context.Context, domain string) { | |||
| 335 | 337 | if hasPNG && !hasSized { | |
| 336 | 338 | log.Debug().Str("domain", domain).Msg("creating sized PNG") | |
| 337 | 339 | if err := a.service.EnsureSizedPNG(ctx, domain, favicon.NormalizedIconSize); err != nil { | |
| 338 | - | a.logWarningDedup(ctx, "sized-png:"+domain, err, func(e *zerolog.Event) { | |
| 339 | - | e.Str("domain", domain).Msg("failed to create sized PNG") | |
| 340 | + | a.logWarningDedup(ctx, "sized-png:"+domain, err, func(log *zerolog.Logger, warnErr error) { | |
| 341 | + | log.Warn().Err(warnErr).Str("domain", domain).Msg("failed to create sized PNG") | |
| 340 | 342 | }) | |
| 341 | 343 | } else { | |
| 342 | 344 | a.clearWarningDedup("sized-png:" + domain) | |
| @@ -365,16 +367,12 @@ func (a *FaviconAdapter) logWarningDedup( | |||
| 365 | 367 | ctx context.Context, | |
| 366 | 368 | key string, | |
| 367 | 369 | err error, | |
| 368 | - | warnFn func(*zerolog.Event), | |
| 370 | + | warnFn warningLogFunc, | |
| 369 | 371 | ) { | |
| 370 | 372 | log := logging.FromContext(ctx) | |
| 371 | 373 | logWarning, suppressed := a.shouldLogWarningDedup(key) | |
| 372 | 374 | if logWarning { | |
| 373 | - | event := log.Warn() | |
| 374 | - | if err != nil { | |
| 375 | - | event = event.Err(err) | |
| 376 | - | } | |
| 377 | - | warnFn(event) | |
| 375 | + | warnFn(log, err) | |
| 378 | 376 | return | |
| 379 | 377 | } | |
| 380 | 378 | ||
| @@ -26,6 +26,8 @@ type FaviconAdapter struct { | |||||
| 26 | warnCounts map[string]int | 26 | warnCounts map[string]int | ||
| 27 | } | 27 | } | ||
| 28 | 28 | ||||
| 29 | + | type warningLogFunc func(log *zerolog.Logger, err error) | |||
| 30 | + | ||||
| 29 | // NewFaviconAdapter creates a new FaviconAdapter. | 31 | // NewFaviconAdapter creates a new FaviconAdapter. | ||
| 30 | // The faviconDB can be nil if WebKit favicon database is not available. | 32 | // The faviconDB can be nil if WebKit favicon database is not available. | ||
| 31 | func NewFaviconAdapter(service *favicon.Service, faviconDB *webkit.FaviconDatabase) *FaviconAdapter { | 33 | func NewFaviconAdapter(service *favicon.Service, faviconDB *webkit.FaviconDatabase) *FaviconAdapter { | ||
| @@ -210,8 +212,8 @@ func (a *FaviconAdapter) saveFaviconToDisk(ctx context.Context, domain string, t | |||||
| 210 | if needsPNGSave { | 212 | if needsPNGSave { | ||
| 211 | // Ensure cache directory exists (cheap check, do it now) | 213 | // Ensure cache directory exists (cheap check, do it now) | ||
| 212 | if err := a.service.EnsureCacheDir(); err != nil { | 214 | if err := a.service.EnsureCacheDir(); err != nil { | ||
| 213 | - | a.logWarningDedup(ctx, cacheDirWarnKey, err, func(e *zerolog.Event) { | 215 | + | a.logWarningDedup(ctx, cacheDirWarnKey, err, func(log *zerolog.Logger, warnErr error) { |
| 214 | - | e.Str("domain", domain).Msg("failed to create favicon cache dir") | 216 | + | log.Warn().Err(warnErr).Str("domain", domain).Msg("failed to create favicon cache dir") |
| 215 | }) | 217 | }) | ||
| 216 | needsPNGSave = false | 218 | needsPNGSave = false | ||
| 217 | } else { | 219 | } else { | ||
| @@ -224,8 +226,8 @@ func (a *FaviconAdapter) saveFaviconToDisk(ctx context.Context, domain string, t | |||||
| 224 | // texture.SaveToPng must run on main thread but we defer it to avoid blocking | 226 | // texture.SaveToPng must run on main thread but we defer it to avoid blocking | ||
| 225 | cb := glib.SourceFunc(func(_ uintptr) bool { | 227 | cb := glib.SourceFunc(func(_ uintptr) bool { | ||
| 226 | if ok := texture.SaveToPng(pngPath); !ok { | 228 | if ok := texture.SaveToPng(pngPath); !ok { | ||
| 227 | - | a.logWarningDedup(ctx, savePNGWarnKey, nil, func(e *zerolog.Event) { | 229 | + | a.logWarningDedup(ctx, savePNGWarnKey, nil, func(log *zerolog.Logger, _ error) { |
| 228 | - | e.Str("domain", domain).Str("path", pngPath).Msg("failed to save favicon PNG") | 230 | + | log.Warn().Str("domain", domain).Str("path", pngPath).Msg("failed to save favicon PNG") |
| 229 | }) | 231 | }) | ||
| 230 | } else { | 232 | } else { | ||
| 231 | a.clearWarningDedup(savePNGWarnKey) | 233 | a.clearWarningDedup(savePNGWarnKey) | ||
| @@ -234,8 +236,8 @@ func (a *FaviconAdapter) saveFaviconToDisk(ctx context.Context, domain string, t | |||||
| 234 | // Create normalized sized copy for dmenu/fuzzel (async) | 236 | // Create normalized sized copy for dmenu/fuzzel (async) | ||
| 235 | go func() { | 237 | go func() { | ||
| 236 | if err := a.service.EnsureSizedPNG(ctx, domain, favicon.NormalizedIconSize); err != nil { | 238 | if err := a.service.EnsureSizedPNG(ctx, domain, favicon.NormalizedIconSize); err != nil { | ||
| 237 | - | a.logWarningDedup(ctx, sizedPNGWarnKey, err, func(e *zerolog.Event) { | 239 | + | a.logWarningDedup(ctx, sizedPNGWarnKey, err, func(log *zerolog.Logger, warnErr error) { |
| 238 | - | e.Str("domain", domain).Msg("failed to create sized PNG") | 240 | + | log.Warn().Err(warnErr).Str("domain", domain).Msg("failed to create sized PNG") |
| 239 | }) | 241 | }) | ||
| 240 | } else { | 242 | } else { | ||
| 241 | a.clearWarningDedup(sizedPNGWarnKey) | 243 | a.clearWarningDedup(sizedPNGWarnKey) | ||
| @@ -335,8 +337,8 @@ func (a *FaviconAdapter) ensureSizedPNG(ctx context.Context, domain string) { | |||||
| 335 | if hasPNG && !hasSized { | 337 | if hasPNG && !hasSized { | ||
| 336 | log.Debug().Str("domain", domain).Msg("creating sized PNG") | 338 | log.Debug().Str("domain", domain).Msg("creating sized PNG") | ||
| 337 | if err := a.service.EnsureSizedPNG(ctx, domain, favicon.NormalizedIconSize); err != nil { | 339 | if err := a.service.EnsureSizedPNG(ctx, domain, favicon.NormalizedIconSize); err != nil { | ||
| 338 | - | a.logWarningDedup(ctx, "sized-png:"+domain, err, func(e *zerolog.Event) { | 340 | + | a.logWarningDedup(ctx, "sized-png:"+domain, err, func(log *zerolog.Logger, warnErr error) { |
| 339 | - | e.Str("domain", domain).Msg("failed to create sized PNG") | 341 | + | log.Warn().Err(warnErr).Str("domain", domain).Msg("failed to create sized PNG") |
| 340 | }) | 342 | }) | ||
| 341 | } else { | 343 | } else { | ||
| 342 | a.clearWarningDedup("sized-png:" + domain) | 344 | a.clearWarningDedup("sized-png:" + domain) | ||
| @@ -365,16 +367,12 @@ func (a *FaviconAdapter) logWarningDedup( | |||||
| 365 | ctx context.Context, | 367 | ctx context.Context, | ||
| 366 | key string, | 368 | key string, | ||
| 367 | err error, | 369 | err error, | ||
| 368 | - | warnFn func(*zerolog.Event), | 370 | + | warnFn warningLogFunc, |
| 369 | ) { | 371 | ) { | ||
| 370 | log := logging.FromContext(ctx) | 372 | log := logging.FromContext(ctx) | ||
| 371 | logWarning, suppressed := a.shouldLogWarningDedup(key) | 373 | logWarning, suppressed := a.shouldLogWarningDedup(key) | ||
| 372 | if logWarning { | 374 | if logWarning { | ||
| 373 | - | event := log.Warn() | 375 | + | warnFn(log, err) |
| 374 | - | if err != nil { | |||
| 375 | - | event = event.Err(err) | |||
| 376 | - | } | |||
| 377 | - | warnFn(event) | |||
| 378 | return | 376 | return | ||
| 379 | } | 377 | } | ||
| 380 | 378 | ||||
M internal/ui/coordinator/oauth_test.go +1 -1
| @@ -106,5 +106,5 @@ func waitFor(t *testing.T, timeout time.Duration, condition func() bool) { | |||
| 106 | 106 | } | |
| 107 | 107 | time.Sleep(20 * time.Millisecond) | |
| 108 | 108 | } | |
| 109 | - | assert.True(t, condition(), "condition not met before timeout") | |
| 109 | + | t.Fatalf("condition not met before timeout (%s)", timeout) | |
| 110 | 110 | } | |
| @@ -106,5 +106,5 @@ func waitFor(t *testing.T, timeout time.Duration, condition func() bool) { | |||||
| 106 | } | 106 | } | ||
| 107 | time.Sleep(20 * time.Millisecond) | 107 | time.Sleep(20 * time.Millisecond) | ||
| 108 | } | 108 | } | ||
| 109 | - | assert.True(t, condition(), "condition not met before timeout") | 109 | + | t.Fatalf("condition not met before timeout (%s)", timeout) |
| 110 | } | 110 | } | ||