fix: sweep marker guard, crash page dedup, version ellipsize, and remove redundant url.Error check
Require both startup and shutdown markers to be aged in sweepPairedMarkers before removing any files. Consolidate duplicate escaped variables in buildCrashPageHTML. Add SetMaxWidthChars/SetEllipsize to loading skeleton version label. Remove unreachable url.Error branch from isRetryableRequestError since *url.Error implements net.Error.
- commit
- 989c49086642a8a86e3d9fb72134073ddade9006
- author
- brice · · 2026-02-07 11:48 UTC
- parent
- d3d4981
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
8 files · +31 -16
M internal/bootstrap/session.go +13 -5
| @@ -493,8 +493,10 @@ func runSessionCleanupAsync( | |||
| 493 | 493 | }() | |
| 494 | 494 | } | |
| 495 | 495 | ||
| 496 | - | // sweepPairedMarkers removes startup, shutdown, and abrupt marker files for | |
| 497 | - | // sessions that have both a startup and shutdown marker older than maxAgeDays. | |
| 496 | + | // sweepPairedMarkers removes marker files for sessions that have both a | |
| 497 | + | // shutdown marker and a startup marker older than maxAgeDays. Both markers | |
| 498 | + | // must exist and be aged past the cutoff before any files are removed. | |
| 499 | + | // The associated abrupt marker (if present) is also cleaned up. | |
| 498 | 500 | // Removal errors are non-fatal (logged and skipped). | |
| 499 | 501 | func sweepPairedMarkers(lockDir string, maxAgeDays int, log *zerolog.Logger) { | |
| 500 | 502 | if lockDir == "" { | |
| @@ -524,15 +526,21 @@ func sweepPairedMarkers(lockDir string, maxAgeDays int, log *zerolog.Logger) { | |||
| 524 | 526 | continue | |
| 525 | 527 | } | |
| 526 | 528 | ||
| 527 | - | info, err := os.Stat(shutdownPath) | |
| 528 | - | if err != nil || info.ModTime().After(cutoff) { | |
| 529 | + | shutdownInfo, err := os.Stat(shutdownPath) | |
| 530 | + | if err != nil || shutdownInfo.ModTime().After(cutoff) { | |
| 529 | 531 | continue | |
| 530 | 532 | } | |
| 531 | 533 | ||
| 532 | 534 | startupPath := startupMarkerPath(lockDir, sessionID) | |
| 535 | + | startupInfo, err := os.Stat(startupPath) | |
| 536 | + | if err != nil || startupInfo.ModTime().After(cutoff) { | |
| 537 | + | // Startup marker missing or too recent — skip this session. | |
| 538 | + | continue | |
| 539 | + | } | |
| 540 | + | ||
| 533 | 541 | abruptPath := abruptMarkerPath(lockDir, sessionID) | |
| 534 | 542 | ||
| 535 | - | // Remove the trio: shutdown, startup (if still present), abrupt (if present). | |
| 543 | + | // Remove the trio: shutdown, startup, abrupt (if present). | |
| 536 | 544 | for _, p := range []string{shutdownPath, startupPath, abruptPath} { | |
| 537 | 545 | if rmErr := os.Remove(p); rmErr != nil && !os.IsNotExist(rmErr) { | |
| 538 | 546 | if log != nil { | |
| @@ -493,8 +493,10 @@ func runSessionCleanupAsync( | |||||
| 493 | }() | 493 | }() | ||
| 494 | } | 494 | } | ||
| 495 | 495 | ||||
| 496 | - | // sweepPairedMarkers removes startup, shutdown, and abrupt marker files for | 496 | + | // sweepPairedMarkers removes marker files for sessions that have both a |
| 497 | - | // sessions that have both a startup and shutdown marker older than maxAgeDays. | 497 | + | // shutdown marker and a startup marker older than maxAgeDays. Both markers |
| 498 | + | // must exist and be aged past the cutoff before any files are removed. | |||
| 499 | + | // The associated abrupt marker (if present) is also cleaned up. | |||
| 498 | // Removal errors are non-fatal (logged and skipped). | 500 | // Removal errors are non-fatal (logged and skipped). | ||
| 499 | func sweepPairedMarkers(lockDir string, maxAgeDays int, log *zerolog.Logger) { | 501 | func sweepPairedMarkers(lockDir string, maxAgeDays int, log *zerolog.Logger) { | ||
| 500 | if lockDir == "" { | 502 | if lockDir == "" { | ||
| @@ -524,15 +526,21 @@ func sweepPairedMarkers(lockDir string, maxAgeDays int, log *zerolog.Logger) { | |||||
| 524 | continue | 526 | continue | ||
| 525 | } | 527 | } | ||
| 526 | 528 | ||||
| 527 | - | info, err := os.Stat(shutdownPath) | 529 | + | shutdownInfo, err := os.Stat(shutdownPath) |
| 528 | - | if err != nil || info.ModTime().After(cutoff) { | 530 | + | if err != nil || shutdownInfo.ModTime().After(cutoff) { |
| 529 | continue | 531 | continue | ||
| 530 | } | 532 | } | ||
| 531 | 533 | ||||
| 532 | startupPath := startupMarkerPath(lockDir, sessionID) | 534 | startupPath := startupMarkerPath(lockDir, sessionID) | ||
| 535 | + | startupInfo, err := os.Stat(startupPath) | |||
| 536 | + | if err != nil || startupInfo.ModTime().After(cutoff) { | |||
| 537 | + | // Startup marker missing or too recent — skip this session. | |||
| 538 | + | continue | |||
| 539 | + | } | |||
| 540 | + | ||||
| 533 | abruptPath := abruptMarkerPath(lockDir, sessionID) | 541 | abruptPath := abruptMarkerPath(lockDir, sessionID) | ||
| 534 | 542 | ||||
| 535 | - | // Remove the trio: shutdown, startup (if still present), abrupt (if present). | 543 | + | // Remove the trio: shutdown, startup, abrupt (if present). |
| 536 | for _, p := range []string{shutdownPath, startupPath, abruptPath} { | 544 | for _, p := range []string{shutdownPath, startupPath, abruptPath} { | ||
| 537 | if rmErr := os.Remove(p); rmErr != nil && !os.IsNotExist(rmErr) { | 545 | if rmErr := os.Remove(p); rmErr != nil && !os.IsNotExist(rmErr) { | ||
| 538 | if log != nil { | 546 | if log != nil { | ||
M internal/infrastructure/updater/github.go +0 -5
| @@ -292,11 +292,6 @@ func isRetryableRequestError(err error) bool { | |||
| 292 | 292 | } | |
| 293 | 293 | } | |
| 294 | 294 | ||
| 295 | - | var urlErr *url.Error | |
| 296 | - | if errors.As(err, &urlErr) && urlErr.Temporary() { | |
| 297 | - | return true | |
| 298 | - | } | |
| 299 | - | ||
| 300 | 295 | return false | |
| 301 | 296 | } | |
| 302 | 297 | ||
| @@ -292,11 +292,6 @@ func isRetryableRequestError(err error) bool { | |||||
| 292 | } | 292 | } | ||
| 293 | } | 293 | } | ||
| 294 | 294 | ||||
| 295 | - | var urlErr *url.Error | |||
| 296 | - | if errors.As(err, &urlErr) && urlErr.Temporary() { | |||
| 297 | - | return true | |||
| 298 | - | } | |||
| 299 | - | ||||
| 300 | return false | 295 | return false | ||
| 301 | } | 296 | } | ||
| 302 | 297 | ||||
M internal/infrastructure/webkit/scheme_handler.go +2 -3
| @@ -233,8 +233,7 @@ func sanitizeCrashPageOriginalURI(originalURI string) string { | |||
| 233 | 233 | } | |
| 234 | 234 | ||
| 235 | 235 | func buildCrashPageHTML(originalURI string) string { | |
| 236 | - | escapedOriginal := html.EscapeString(originalURI) | |
| 237 | - | escapedReloadTarget := html.EscapeString(originalURI) | |
| 236 | + | escapedURI := html.EscapeString(originalURI) | |
| 238 | 237 | return fmt.Sprintf(`<!DOCTYPE html> | |
| 239 | 238 | <html lang="en"> | |
| 240 | 239 | <head> | |
| @@ -333,7 +332,7 @@ func buildCrashPageHTML(originalURI string) string { | |||
| 333 | 332 | }); | |
| 334 | 333 | </script> | |
| 335 | 334 | </body> | |
| 336 | - | </html>`, escapedOriginal, escapedReloadTarget) | |
| 335 | + | </html>`, escapedURI, escapedURI) | |
| 337 | 336 | } | |
| 338 | 337 | ||
| 339 | 338 | func (h *DumbSchemeHandler) buildConfigResponse(cfg *config.Config) *SchemeResponse { | |
| @@ -233,8 +233,7 @@ func sanitizeCrashPageOriginalURI(originalURI string) string { | |||||
| 233 | } | 233 | } | ||
| 234 | 234 | ||||
| 235 | func buildCrashPageHTML(originalURI string) string { | 235 | func buildCrashPageHTML(originalURI string) string { | ||
| 236 | - | escapedOriginal := html.EscapeString(originalURI) | 236 | + | escapedURI := html.EscapeString(originalURI) |
| 237 | - | escapedReloadTarget := html.EscapeString(originalURI) | |||
| 238 | return fmt.Sprintf(`<!DOCTYPE html> | 237 | return fmt.Sprintf(`<!DOCTYPE html> | ||
| 239 | <html lang="en"> | 238 | <html lang="en"> | ||
| 240 | <head> | 239 | <head> | ||
| @@ -333,7 +332,7 @@ func buildCrashPageHTML(originalURI string) string { | |||||
| 333 | }); | 332 | }); | ||
| 334 | </script> | 333 | </script> | ||
| 335 | </body> | 334 | </body> | ||
| 336 | - | </html>`, escapedOriginal, escapedReloadTarget) | 335 | + | </html>`, escapedURI, escapedURI) |
| 337 | } | 336 | } | ||
| 338 | 337 | ||||
| 339 | func (h *DumbSchemeHandler) buildConfigResponse(cfg *config.Config) *SchemeResponse { | 338 | func (h *DumbSchemeHandler) buildConfigResponse(cfg *config.Config) *SchemeResponse { | ||
M internal/ui/component/loading_skeleton.go +6 -3
| @@ -19,9 +19,10 @@ type LoadingSkeleton struct { | |||
| 19 | 19 | } | |
| 20 | 20 | ||
| 21 | 21 | const ( | |
| 22 | - | loadingSkeletonSpacing = 6 | |
| 23 | - | loadingSkeletonLogoSize = 256 | |
| 24 | - | loadingSkeletonSpinnerSize = 32 | |
| 22 | + | loadingSkeletonSpacing = 6 | |
| 23 | + | loadingSkeletonLogoSize = 256 | |
| 24 | + | loadingSkeletonSpinnerSize = 32 | |
| 25 | + | loadingSkeletonVersionMaxChars = 16 | |
| 25 | 26 | ) | |
| 26 | 27 | ||
| 27 | 28 | // skeletonVersion is set via SetSkeletonVersion during startup. | |
| @@ -83,6 +84,8 @@ func NewLoadingSkeleton(factory layout.WidgetFactory) *LoadingSkeleton { | |||
| 83 | 84 | versionLabel.SetValign(gtk.AlignCenterValue) | |
| 84 | 85 | versionLabel.SetCanFocus(false) | |
| 85 | 86 | versionLabel.SetCanTarget(false) | |
| 87 | + | versionLabel.SetMaxWidthChars(loadingSkeletonVersionMaxChars) | |
| 88 | + | versionLabel.SetEllipsize(layout.EllipsizeEnd) | |
| 86 | 89 | versionLabel.AddCssClass("loading-skeleton-version") | |
| 87 | 90 | ||
| 88 | 91 | // Layout: logo, spinner, version (vertically centered) | |
| @@ -19,9 +19,10 @@ type LoadingSkeleton struct { | |||||
| 19 | } | 19 | } | ||
| 20 | 20 | ||||
| 21 | const ( | 21 | const ( | ||
| 22 | - | loadingSkeletonSpacing = 6 | 22 | + | loadingSkeletonSpacing = 6 |
| 23 | - | loadingSkeletonLogoSize = 256 | 23 | + | loadingSkeletonLogoSize = 256 |
| 24 | - | loadingSkeletonSpinnerSize = 32 | 24 | + | loadingSkeletonSpinnerSize = 32 |
| 25 | + | loadingSkeletonVersionMaxChars = 16 | |||
| 25 | ) | 26 | ) | ||
| 26 | 27 | ||||
| 27 | // skeletonVersion is set via SetSkeletonVersion during startup. | 28 | // skeletonVersion is set via SetSkeletonVersion during startup. | ||
| @@ -83,6 +84,8 @@ func NewLoadingSkeleton(factory layout.WidgetFactory) *LoadingSkeleton { | |||||
| 83 | versionLabel.SetValign(gtk.AlignCenterValue) | 84 | versionLabel.SetValign(gtk.AlignCenterValue) | ||
| 84 | versionLabel.SetCanFocus(false) | 85 | versionLabel.SetCanFocus(false) | ||
| 85 | versionLabel.SetCanTarget(false) | 86 | versionLabel.SetCanTarget(false) | ||
| 87 | + | versionLabel.SetMaxWidthChars(loadingSkeletonVersionMaxChars) | |||
| 88 | + | versionLabel.SetEllipsize(layout.EllipsizeEnd) | |||
| 86 | versionLabel.AddCssClass("loading-skeleton-version") | 89 | versionLabel.AddCssClass("loading-skeleton-version") | ||
| 87 | 90 | ||||
| 88 | // Layout: logo, spinner, version (vertically centered) | 91 | // Layout: logo, spinner, version (vertically centered) | ||
M internal/ui/component/loading_skeleton_test.go +4 -0
| @@ -58,6 +58,8 @@ func TestNewLoadingSkeleton_ShowsAndStartsSpinner(t *testing.T) { | |||
| 58 | 58 | mockVersion.EXPECT().SetValign(mock.Anything).Once() | |
| 59 | 59 | mockVersion.EXPECT().SetCanFocus(false).Once() | |
| 60 | 60 | mockVersion.EXPECT().SetCanTarget(false).Once() | |
| 61 | + | mockVersion.EXPECT().SetMaxWidthChars(mock.Anything).Once() | |
| 62 | + | mockVersion.EXPECT().SetEllipsize(mock.Anything).Once() | |
| 61 | 63 | mockVersion.EXPECT().AddCssClass("loading-skeleton-version").Once() | |
| 62 | 64 | ||
| 63 | 65 | mockContent.EXPECT().Append(mockLogo).Once() | |
| @@ -120,6 +122,8 @@ func TestLoadingSkeleton_SetVisible_StopsSpinnerWhenHidden(t *testing.T) { | |||
| 120 | 122 | mockVersion.EXPECT().SetValign(mock.Anything).Once() | |
| 121 | 123 | mockVersion.EXPECT().SetCanFocus(false).Once() | |
| 122 | 124 | mockVersion.EXPECT().SetCanTarget(false).Once() | |
| 125 | + | mockVersion.EXPECT().SetMaxWidthChars(mock.Anything).Once() | |
| 126 | + | mockVersion.EXPECT().SetEllipsize(mock.Anything).Once() | |
| 123 | 127 | mockVersion.EXPECT().AddCssClass("loading-skeleton-version").Once() | |
| 124 | 128 | ||
| 125 | 129 | mockContent.EXPECT().Append(mockLogo).Once() | |
| @@ -58,6 +58,8 @@ func TestNewLoadingSkeleton_ShowsAndStartsSpinner(t *testing.T) { | |||||
| 58 | mockVersion.EXPECT().SetValign(mock.Anything).Once() | 58 | mockVersion.EXPECT().SetValign(mock.Anything).Once() | ||
| 59 | mockVersion.EXPECT().SetCanFocus(false).Once() | 59 | mockVersion.EXPECT().SetCanFocus(false).Once() | ||
| 60 | mockVersion.EXPECT().SetCanTarget(false).Once() | 60 | mockVersion.EXPECT().SetCanTarget(false).Once() | ||
| 61 | + | mockVersion.EXPECT().SetMaxWidthChars(mock.Anything).Once() | |||
| 62 | + | mockVersion.EXPECT().SetEllipsize(mock.Anything).Once() | |||
| 61 | mockVersion.EXPECT().AddCssClass("loading-skeleton-version").Once() | 63 | mockVersion.EXPECT().AddCssClass("loading-skeleton-version").Once() | ||
| 62 | 64 | ||||
| 63 | mockContent.EXPECT().Append(mockLogo).Once() | 65 | mockContent.EXPECT().Append(mockLogo).Once() | ||
| @@ -120,6 +122,8 @@ func TestLoadingSkeleton_SetVisible_StopsSpinnerWhenHidden(t *testing.T) { | |||||
| 120 | mockVersion.EXPECT().SetValign(mock.Anything).Once() | 122 | mockVersion.EXPECT().SetValign(mock.Anything).Once() | ||
| 121 | mockVersion.EXPECT().SetCanFocus(false).Once() | 123 | mockVersion.EXPECT().SetCanFocus(false).Once() | ||
| 122 | mockVersion.EXPECT().SetCanTarget(false).Once() | 124 | mockVersion.EXPECT().SetCanTarget(false).Once() | ||
| 125 | + | mockVersion.EXPECT().SetMaxWidthChars(mock.Anything).Once() | |||
| 126 | + | mockVersion.EXPECT().SetEllipsize(mock.Anything).Once() | |||
| 123 | mockVersion.EXPECT().AddCssClass("loading-skeleton-version").Once() | 127 | mockVersion.EXPECT().AddCssClass("loading-skeleton-version").Once() | ||
| 124 | 128 | ||||
| 125 | mockContent.EXPECT().Append(mockLogo).Once() | 129 | mockContent.EXPECT().Append(mockLogo).Once() | ||
M internal/ui/component/pane_view_internal_test.go +2 -0
| @@ -56,6 +56,8 @@ func TestPaneView_HideLoadingSkeleton_HidesWhenPresent(t *testing.T) { | |||
| 56 | 56 | mockVersion.EXPECT().SetValign(mock.Anything).Once() | |
| 57 | 57 | mockVersion.EXPECT().SetCanFocus(false).Once() | |
| 58 | 58 | mockVersion.EXPECT().SetCanTarget(false).Once() | |
| 59 | + | mockVersion.EXPECT().SetMaxWidthChars(mock.Anything).Once() | |
| 60 | + | mockVersion.EXPECT().SetEllipsize(mock.Anything).Once() | |
| 59 | 61 | mockVersion.EXPECT().AddCssClass("loading-skeleton-version").Once() | |
| 60 | 62 | ||
| 61 | 63 | mockContent.EXPECT().Append(mockLogo).Once() | |
| @@ -56,6 +56,8 @@ func TestPaneView_HideLoadingSkeleton_HidesWhenPresent(t *testing.T) { | |||||
| 56 | mockVersion.EXPECT().SetValign(mock.Anything).Once() | 56 | mockVersion.EXPECT().SetValign(mock.Anything).Once() | ||
| 57 | mockVersion.EXPECT().SetCanFocus(false).Once() | 57 | mockVersion.EXPECT().SetCanFocus(false).Once() | ||
| 58 | mockVersion.EXPECT().SetCanTarget(false).Once() | 58 | mockVersion.EXPECT().SetCanTarget(false).Once() | ||
| 59 | + | mockVersion.EXPECT().SetMaxWidthChars(mock.Anything).Once() | |||
| 60 | + | mockVersion.EXPECT().SetEllipsize(mock.Anything).Once() | |||
| 59 | mockVersion.EXPECT().AddCssClass("loading-skeleton-version").Once() | 61 | mockVersion.EXPECT().AddCssClass("loading-skeleton-version").Once() | ||
| 60 | 62 | ||||
| 61 | mockContent.EXPECT().Append(mockLogo).Once() | 63 | mockContent.EXPECT().Append(mockLogo).Once() | ||
M internal/ui/component/pane_view_test.go +2 -0
| @@ -560,6 +560,8 @@ func setupLoadingSkeletonMocks( | |||
| 560 | 560 | mockLoadingVersion.EXPECT().SetValign(mock.Anything).Maybe() | |
| 561 | 561 | mockLoadingVersion.EXPECT().SetCanFocus(false).Maybe() | |
| 562 | 562 | mockLoadingVersion.EXPECT().SetCanTarget(false).Maybe() | |
| 563 | + | mockLoadingVersion.EXPECT().SetMaxWidthChars(mock.Anything).Maybe() | |
| 564 | + | mockLoadingVersion.EXPECT().SetEllipsize(mock.Anything).Maybe() | |
| 563 | 565 | mockLoadingVersion.EXPECT().AddCssClass("loading-skeleton-version").Maybe() | |
| 564 | 566 | ||
| 565 | 567 | mockLoadingContent.EXPECT().Append(mockLoadingLogo).Maybe() | |
| @@ -560,6 +560,8 @@ func setupLoadingSkeletonMocks( | |||||
| 560 | mockLoadingVersion.EXPECT().SetValign(mock.Anything).Maybe() | 560 | mockLoadingVersion.EXPECT().SetValign(mock.Anything).Maybe() | ||
| 561 | mockLoadingVersion.EXPECT().SetCanFocus(false).Maybe() | 561 | mockLoadingVersion.EXPECT().SetCanFocus(false).Maybe() | ||
| 562 | mockLoadingVersion.EXPECT().SetCanTarget(false).Maybe() | 562 | mockLoadingVersion.EXPECT().SetCanTarget(false).Maybe() | ||
| 563 | + | mockLoadingVersion.EXPECT().SetMaxWidthChars(mock.Anything).Maybe() | |||
| 564 | + | mockLoadingVersion.EXPECT().SetEllipsize(mock.Anything).Maybe() | |||
| 563 | mockLoadingVersion.EXPECT().AddCssClass("loading-skeleton-version").Maybe() | 565 | mockLoadingVersion.EXPECT().AddCssClass("loading-skeleton-version").Maybe() | ||
| 564 | 566 | ||||
| 565 | mockLoadingContent.EXPECT().Append(mockLoadingLogo).Maybe() | 567 | mockLoadingContent.EXPECT().Append(mockLoadingLogo).Maybe() | ||
M internal/ui/component/workspace_view_test.go +2 -0
| @@ -66,6 +66,8 @@ func setupLoadingSkeletonMocksWorkspace( | |||
| 66 | 66 | mockLoadingVersion.EXPECT().SetValign(mock.Anything).Maybe() | |
| 67 | 67 | mockLoadingVersion.EXPECT().SetCanFocus(false).Maybe() | |
| 68 | 68 | mockLoadingVersion.EXPECT().SetCanTarget(false).Maybe() | |
| 69 | + | mockLoadingVersion.EXPECT().SetMaxWidthChars(mock.Anything).Maybe() | |
| 70 | + | mockLoadingVersion.EXPECT().SetEllipsize(mock.Anything).Maybe() | |
| 69 | 71 | mockLoadingVersion.EXPECT().AddCssClass("loading-skeleton-version").Maybe() | |
| 70 | 72 | ||
| 71 | 73 | mockLoadingContent.EXPECT().Append(mockLoadingLogo).Maybe() | |
| @@ -66,6 +66,8 @@ func setupLoadingSkeletonMocksWorkspace( | |||||
| 66 | mockLoadingVersion.EXPECT().SetValign(mock.Anything).Maybe() | 66 | mockLoadingVersion.EXPECT().SetValign(mock.Anything).Maybe() | ||
| 67 | mockLoadingVersion.EXPECT().SetCanFocus(false).Maybe() | 67 | mockLoadingVersion.EXPECT().SetCanFocus(false).Maybe() | ||
| 68 | mockLoadingVersion.EXPECT().SetCanTarget(false).Maybe() | 68 | mockLoadingVersion.EXPECT().SetCanTarget(false).Maybe() | ||
| 69 | + | mockLoadingVersion.EXPECT().SetMaxWidthChars(mock.Anything).Maybe() | |||
| 70 | + | mockLoadingVersion.EXPECT().SetEllipsize(mock.Anything).Maybe() | |||
| 69 | mockLoadingVersion.EXPECT().AddCssClass("loading-skeleton-version").Maybe() | 71 | mockLoadingVersion.EXPECT().AddCssClass("loading-skeleton-version").Maybe() | ||
| 70 | 72 | ||||
| 71 | mockLoadingContent.EXPECT().Append(mockLoadingLogo).Maybe() | 73 | mockLoadingContent.EXPECT().Append(mockLoadingLogo).Maybe() | ||