diff --git a/server/internal/checkers/checkbrowser/checker.go b/server/internal/checkers/checkbrowser/checker.go index 361438f8b..11c31d485 100644 --- a/server/internal/checkers/checkbrowser/checker.go +++ b/server/internal/checkers/checkbrowser/checker.go @@ -9,6 +9,7 @@ import ( "strings" "time" + "github.com/chromedp/cdproto" "github.com/chromedp/chromedp" checkconfig "github.com/fclairamb/solidping/server/internal/checkers/checkbrowser/config" @@ -329,8 +330,19 @@ func (c *BrowserChecker) captureScreenshot( shot, err := capture(shotCtx) if err != nil { - slog.WarnContext(ctx, "browser check: screenshot capture failed", - "url", cfg.URL, "error", err, "budget_remaining_ms", budgetRemaining.Milliseconds()) + // A page that closed or crashed before the shot is the monitored + // page's doing, not a CDP/infra failure: logged at INFO so it does not + // read as a worker fault in the pod logs. + detached := pageDetached(err) + + level := slog.LevelWarn + if detached { + level = slog.LevelInfo + } + + slog.Log(ctx, level, "browser check: screenshot capture failed", + "url", cfg.URL, "error", err, "budget_remaining_ms", budgetRemaining.Milliseconds(), + "page_detached", detached) setScreenshotError(result, CaptureErrorMessage(err, budgetRemaining)) return @@ -387,9 +399,31 @@ func CaptureErrorMessage(err error, budget time.Duration) string { return "the capture timed out" } + if pageDetached(err) { + return "the page was closed or crashed before the capture" + } + return err.Error() } +// cdpServerError is the JSON-RPC "server error" code Chrome answers with when +// a command targets a session whose page is gone. +const cdpServerError = -32000 + +// pageDetached reports whether a CDP command failed because its page was no +// longer attached: the tab closed itself, crashed, or the target went away +// before the command ran. Chrome reports it as -32000 "Not attached to an +// active page" (or "Session with given id not found"). +func pageDetached(err error) bool { + var cdpErr *cdproto.Error + if !errors.As(err, &cdpErr) || cdpErr.Code != cdpServerError { + return false + } + + return strings.Contains(cdpErr.Message, "Not attached to an active page") || + strings.Contains(cdpErr.Message, "Session with given id not found") +} + // OverCapMessage is the ScreenshotError of a capture dropped over // MaxScreenshotBytes. Shared with the JS runtime. func OverCapMessage(size int) string { diff --git a/server/internal/checkers/checkbrowser/page_detached_test.go b/server/internal/checkers/checkbrowser/page_detached_test.go new file mode 100644 index 000000000..8a9a85631 --- /dev/null +++ b/server/internal/checkers/checkbrowser/page_detached_test.go @@ -0,0 +1,54 @@ +package checkbrowser + +import ( + "context" + "errors" + "fmt" + "testing" + + "github.com/chromedp/cdproto" + "github.com/stretchr/testify/require" +) + +// errPlainDetachedText carries the CDP wording without being a CDP error. +var errPlainDetachedText = errors.New("plain: Not attached to an active page") + +// TestCaptureErrorMessageNamesADetachedPage: a capture whose page closed or +// crashed first fails with CDP -32000 "Not attached to an active page". That +// is the monitored page's doing, so the operator reads a plain sentence rather +// than the raw protocol error, and the log is not raised as a worker fault. +func TestCaptureErrorMessageNamesADetachedPage(t *testing.T) { + t.Parallel() + + detached := &cdproto.Error{Code: cdpServerError, Message: "Not attached to an active page"} + gone := &cdproto.Error{Code: cdpServerError, Message: "Session with given id not found"} + other := &cdproto.Error{Code: cdpServerError, Message: "Internal error"} + + cases := []struct { + name string + err error + detached bool + message string + }{ + {"detached", detached, true, "the page was closed or crashed before the capture"}, + {"wrapped", fmt.Errorf("capture: %w", detached), true, "the page was closed or crashed before the capture"}, + {"session gone", gone, true, "the page was closed or crashed before the capture"}, + {"other cdp error", other, false, other.Error()}, + {"plain error", errRendererGone, false, errRendererGone.Error()}, + {"deadline", context.DeadlineExceeded, false, "the capture timed out"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + r := require.New(t) + + r.Equal(tc.detached, pageDetached(tc.err)) + r.Equal(tc.message, CaptureErrorMessage(tc.err, 0)) + }) + } + + r := require.New(t) + r.False(pageDetached(nil)) + r.False(pageDetached(errPlainDetachedText)) +} diff --git a/server/internal/checkers/checksftp/checker.go b/server/internal/checkers/checksftp/checker.go index a3502623c..442c625eb 100644 --- a/server/internal/checkers/checksftp/checker.go +++ b/server/internal/checkers/checksftp/checker.go @@ -125,6 +125,13 @@ func (c *SFTPChecker) Execute(ctx context.Context, config checkerdef.Config) (*c }, nil } + // ssh.ClientConfig.Timeout only bounds a dial ssh does itself, and the + // handshake, the SFTP session and Stat read this conn with no deadline: a + // server that stalls after accepting would outlive the worker's deadline. + // Closing the conn when the context ends unblocks every one of them. + stopWatch := context.AfterFunc(ctx, func() { _ = netConn.Close() }) + defer stopWatch() + sshConn, chans, reqs, err := ssh.NewClientConn(netConn, target, sshConfig) if err != nil { _ = netConn.Close() @@ -164,9 +171,33 @@ func (c *SFTPChecker) Execute(ctx context.Context, config checkerdef.Config) (*c connectTime := time.Since(connectStart) metrics["connection_time_ms"] = float64(connectTime.Microseconds()) / microsecondsPerMs + if failed := probeSFTP(ctx, sshClient, cfg.Path, start, metrics, output); failed != nil { + return failed, nil + } + + metrics["total_time_ms"] = float64(time.Since(start).Microseconds()) / microsecondsPerMs + + return &checkerdef.Result{ + Status: checkerdef.StatusUp, + Duration: time.Since(start), + Metrics: metrics, + Output: output, + }, nil +} + +// probeSFTP opens the SFTP session and, when path is set, stats it. It returns +// the failing result, or nil when both succeeded. +func probeSFTP( + ctx context.Context, sshClient *ssh.Client, path string, + start time.Time, metrics, output map[string]any, +) *checkerdef.Result { // Create SFTP client sftpClient, err := sftp.NewClient(sshClient) if err != nil { + if ctx.Err() != nil { + return timeoutResult(start, metrics, output, "SFTP session timeout") + } + return &checkerdef.Result{ Status: checkerdef.StatusDown, Duration: time.Since(start), @@ -174,15 +205,19 @@ func (c *SFTPChecker) Execute(ctx context.Context, config checkerdef.Config) (*c Output: mergeOutput(output, map[string]any{ checkerdef.OutputKeyError: fmt.Sprintf("SFTP session failed: %v", err), }), - }, nil + } } defer func() { _ = sftpClient.Close() }() // Verify path if specified - if cfg.Path != "" { - _, err = sftpClient.Stat(cfg.Path) + if path != "" { + _, err = sftpClient.Stat(path) if err != nil { + if ctx.Err() != nil { + return timeoutResult(start, metrics, output, "path verification timeout") + } + return &checkerdef.Result{ Status: checkerdef.StatusDown, Duration: time.Since(start), @@ -190,18 +225,11 @@ func (c *SFTPChecker) Execute(ctx context.Context, config checkerdef.Config) (*c Output: mergeOutput(output, map[string]any{ checkerdef.OutputKeyError: fmt.Sprintf("path verification failed: %v", err), }), - }, nil + } } } - metrics["total_time_ms"] = float64(time.Since(start).Microseconds()) / microsecondsPerMs - - return &checkerdef.Result{ - Status: checkerdef.StatusUp, - Duration: time.Since(start), - Metrics: metrics, - Output: output, - }, nil + return nil } // hostKeyCallback builds the SSH host key callback for an SFTP check. @@ -225,6 +253,15 @@ func hostKeyCallback(pin string, output map[string]any, observed *string) ssh.Ho } } +func timeoutResult(start time.Time, metrics, output map[string]any, msg string) *checkerdef.Result { + return &checkerdef.Result{ + Status: checkerdef.StatusTimeout, + Duration: time.Since(start), + Metrics: metrics, + Output: mergeOutput(output, map[string]any{checkerdef.OutputKeyError: msg}), + } +} + func mergeOutput(base, extra map[string]any) map[string]any { result := make(map[string]any, len(base)+len(extra)) for k, v := range base { diff --git a/server/internal/checkers/checksftp/context_test.go b/server/internal/checkers/checksftp/context_test.go new file mode 100644 index 000000000..b976053ef --- /dev/null +++ b/server/internal/checkers/checksftp/context_test.go @@ -0,0 +1,72 @@ +package checksftp_test + +import ( + "context" + "net" + "strconv" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/fclairamb/solidping/server/internal/checkers/checkerdef" + "github.com/fclairamb/solidping/server/internal/checkers/checksftp" +) + +// TestExecute_HonorsContextOnAStalledServer: a server that accepts the TCP +// connection and then never speaks used to hold the checker in the SSH +// handshake until the OS gave up, past the worker's deadline, so the worker +// abandoned it ("did not honor context"). Canceling the context must now cut +// the connection and return a timeout promptly. +func TestExecute_HonorsContextOnAStalledServer(t *testing.T) { + t.Parallel() + r := require.New(t) + + listener, err := (&net.ListenConfig{}).Listen(t.Context(), "tcp", "127.0.0.1:0") + r.NoError(err) + + t.Cleanup(func() { _ = listener.Close() }) + + go func() { + for { + conn, acceptErr := listener.Accept() + if acceptErr != nil { + return + } + + t.Cleanup(func() { _ = conn.Close() }) // held open, silent + } + }() + + host, portStr, err := net.SplitHostPort(listener.Addr().String()) + r.NoError(err) + + port, err := strconv.Atoi(portStr) + r.NoError(err) + + cfg := &checksftp.SFTPConfig{ + Host: host, + Port: port, + Timeout: time.Minute, // the context, not the config, must stop it + Username: "tester", + Password: "s3cret", + } + + ctx, cancel := context.WithTimeout(t.Context(), 300*time.Millisecond) + defer cancel() + + done := make(chan *checkerdef.Result, 1) + + go func() { + result, _ := (&checksftp.SFTPChecker{}).Execute(ctx, cfg) + done <- result + }() + + select { + case result := <-done: + r.NotNil(result) + r.Equal(checkerdef.StatusTimeout, result.Status, "output: %#v", result.Output) + case <-time.After(5 * time.Second): + r.Fail("Execute ignored its context on a stalled SSH handshake") + } +} diff --git a/web/dash0/src/i18n-html-lang.test.ts b/web/dash0/src/i18n-html-lang.test.ts new file mode 100644 index 000000000..87a43eae9 --- /dev/null +++ b/web/dash0/src/i18n-html-lang.test.ts @@ -0,0 +1,34 @@ +/** + * @vitest-environment jsdom + */ +import { afterEach, describe, expect, it } from "vitest"; +import i18n, { i18nReady } from "./i18n"; + +// The static index.html says lang="en". When the app renders another +// language, a stale lang makes the browser offer to auto-translate a page +// that is already in the visitor's language, and the translator's DOM edits +// then crash React reconciliation (removeChild NotFoundError). +describe(" follows the rendered language", () => { + afterEach(async () => { + await i18n.changeLanguage("en"); + }); + + it("is the rendered language after a change", async () => { + await i18nReady; + await i18n.changeLanguage("es"); + expect(document.documentElement.lang).toBe("es"); + + await i18n.changeLanguage("en"); + expect(document.documentElement.lang).toBe("en"); + }); + + it("uses the base language for a regional variant", async () => { + await i18n.changeLanguage("fr-CA"); + expect(document.documentElement.lang).toBe("fr"); + }); + + it("stays English for a language the app does not ship", async () => { + await i18n.changeLanguage("pt-BR"); + expect(document.documentElement.lang).toBe("en"); + }); +}); diff --git a/web/dash0/src/i18n.ts b/web/dash0/src/i18n.ts index a7c240e2c..50bb9410f 100644 --- a/web/dash0/src/i18n.ts +++ b/web/dash0/src/i18n.ts @@ -89,6 +89,16 @@ const enResources = { support: en_support, }; +// Keep on the language actually rendered (index.html ships "en"). +// A stale lang makes the browser offer to auto-translate a page already in the +// visitor's language, and the translator's DOM edits crash React's +// reconciliation (removeChild NotFoundError). resolvedLanguage is the shipped +// language in use: "fr" for fr-CA, "en" for a language the app lacks. +i18n.on("languageChanged", () => { + if (typeof document === "undefined") return; + document.documentElement.lang = i18n.resolvedLanguage ?? "en"; +}); + const initPromise = i18n .use(LanguageDetector) .use(initReactI18next)