Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 25 additions & 6 deletions server/cmd/api/api/chromium_configure.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,13 @@ import (

const userDataProfileDir = "/home/kernel/user-data"
const maxStartURLLen = 2048
const startURLDispatchTimeout = 3 * time.Second

// startURLNavigateTimeout bounds the start_url navigation. Page.navigate only
// resolves once the navigation commits, so this budget has to cover the origin's
// DNS, connect and first byte; the previous 3s budget expired against slow
// origins and handed the session over still showing the pre-navigation page. It
// does not have to cover subresource loading, which continues after commit.
const startURLNavigateTimeout = 15 * time.Second

type chromiumConfigureState struct {
displayJSON *string
Expand Down Expand Up @@ -181,8 +187,19 @@ func (s *ApiService) ChromiumConfigure(ctx context.Context, request oapi.Chromiu
}

if spec.needsNav {
if err := chromiumDoNavigate(ctx, s, spec); err != nil {
logger.FromContext(ctx).Warn("start_url dispatch failed", "error", err)
// A failed navigation leaves the session on whatever the browser was
// showing, so log it and keep going rather than discarding a browser over
// a start_url the caller may not control.
navStart := time.Now()
navErrorText, navErr := chromiumDoNavigate(ctx, s, spec)
if navErr == nil && navErrorText != "" {
navErr = errors.New(navErrorText)
}
if navErr != nil {
logger.FromContext(ctx).Warn("start_url dispatch failed",
"error", navErr,
"elapsed", time.Since(navStart).String(),
"timeout", startURLNavigateTimeout.String())
}
}

Expand Down Expand Up @@ -234,12 +251,14 @@ func normalizeStartURL(rawURL string) string {
return rawURL
}

func chromiumDoNavigate(ctx context.Context, s *ApiService, spec startURLParsed) error {
// chromiumDoNavigate navigates to spec.url and returns Chrome's navigation
// errorText, which is non-empty when an error page committed instead.
func chromiumDoNavigate(ctx context.Context, s *ApiService, spec startURLParsed) (string, error) {
upstream := s.upstreamMgr.Current()
if upstream == "" {
return fmt.Errorf("devtools upstream not available")
return "", fmt.Errorf("devtools upstream not available")
}
navCtx, cancel := context.WithTimeout(ctx, startURLDispatchTimeout)
navCtx, cancel := context.WithTimeout(ctx, startURLNavigateTimeout)
defer cancel()
return cdpclient.DispatchStartURL(navCtx, upstream, spec.url)
}
Expand Down
2 changes: 1 addition & 1 deletion server/cmd/wrapper/snapshot_start_page.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func prepareSnapshotStartPage(ctx context.Context, internalPort string) (retErr
logf("WARNING: snapshot start page unavailable, using about:blank: %v", navErr)
blankCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
defer cancel()
if err := cdpclient.DispatchStartURL(blankCtx, devtoolsURL, "about:blank"); err != nil {
if _, err := cdpclient.DispatchStartURL(blankCtx, devtoolsURL, "about:blank"); err != nil {
return fmt.Errorf("reset snapshot start page: %w", err)
}
return nil
Expand Down
46 changes: 31 additions & 15 deletions server/lib/cdpclient/cdpclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,19 +220,26 @@ func (c *Client) CountPageTargets(ctx context.Context) (int, error) {
return n, nil
}

// DispatchStartURL closes extra page targets and dispatches a navigation on the
// first page target. It does not wait for lifecycle events; Chrome owns the
// eventual navigation result.
func DispatchStartURL(ctx context.Context, devtoolsURL, url string) error {
// DispatchStartURL closes extra page targets and navigates the first page
// target. Page.navigate resolves once the navigation commits, so this returns
// after the new document has replaced the old one but before subresources
// finish loading.
//
// The first return value is Chrome's navigation errorText, non-empty when the
// navigation committed an error page instead of the requested URL (an
// unreachable host, for example). Callers that need the requested page to have
// actually loaded must check it; a nil error alone only means the command was
// accepted.
func DispatchStartURL(ctx context.Context, devtoolsURL, url string) (string, error) {
c, err := Dial(ctx, devtoolsURL)
if err != nil {
return fmt.Errorf("dial devtools: %w", err)
return "", fmt.Errorf("dial devtools: %w", err)
}
defer c.Close()

targetsResult, err := c.send(ctx, "Target.getTargets", nil, "")
if err != nil {
return fmt.Errorf("Target.getTargets: %w", err)
return "", fmt.Errorf("Target.getTargets: %w", err)
}

var targets struct {
Expand All @@ -242,7 +249,7 @@ func DispatchStartURL(ctx context.Context, devtoolsURL, url string) error {
} `json:"targetInfos"`
}
if err := json.Unmarshal(targetsResult, &targets); err != nil {
return fmt.Errorf("unmarshal targets: %w", err)
return "", fmt.Errorf("unmarshal targets: %w", err)
}

var pageTargetID string
Expand All @@ -263,13 +270,13 @@ func DispatchStartURL(ctx context.Context, devtoolsURL, url string) error {
"url": "about:blank",
}, "")
if err != nil {
return fmt.Errorf("Target.createTarget: %w", err)
return "", fmt.Errorf("Target.createTarget: %w", err)
}
var created struct {
TargetID string `json:"targetId"`
}
if err := json.Unmarshal(createResult, &created); err != nil {
return fmt.Errorf("unmarshal create target: %w", err)
return "", fmt.Errorf("unmarshal create target: %w", err)
}
pageTargetID = created.TargetID
}
Expand All @@ -279,14 +286,14 @@ func DispatchStartURL(ctx context.Context, devtoolsURL, url string) error {
"flatten": true,
}, "")
if err != nil {
return fmt.Errorf("Target.attachToTarget: %w", err)
return "", fmt.Errorf("Target.attachToTarget: %w", err)
}

var attach struct {
SessionID string `json:"sessionId"`
}
if err := json.Unmarshal(attachResult, &attach); err != nil {
return fmt.Errorf("unmarshal attach: %w", err)
return "", fmt.Errorf("unmarshal attach: %w", err)
}
defer func() {
detachCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
Expand All @@ -296,16 +303,25 @@ func DispatchStartURL(ctx context.Context, devtoolsURL, url string) error {
}, "")
}()

if _, err := c.send(ctx, "Page.navigate", map[string]any{"url": url}, attach.SessionID); err != nil {
return fmt.Errorf("Page.navigate: %w", err)
navResult, err := c.send(ctx, "Page.navigate", map[string]any{"url": url}, attach.SessionID)
if err != nil {
return "", fmt.Errorf("Page.navigate: %w", err)
}
return nil
var nav struct {
ErrorText string `json:"errorText"`
}
if err := json.Unmarshal(navResult, &nav); err != nil {
return "", fmt.Errorf("unmarshal navigate result: %w", err)
}
return nav.ErrorText, nil
}

// DispatchStartURLAndWait navigates through navigationURL and waits for
// destination to load without resolving to Chrome's network error page.
func DispatchStartURLAndWait(ctx context.Context, devtoolsURL, navigationURL, destination string) error {
if err := DispatchStartURL(ctx, devtoolsURL, navigationURL); err != nil {
// The polling loop below re-navigates through chrome-error:// pages, so a
// reported navigation error is not terminal here.
if _, err := DispatchStartURL(ctx, devtoolsURL, navigationURL); err != nil {
return err
}

Expand Down
65 changes: 64 additions & 1 deletion server/lib/cdpclient/cdpclient_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ type fakeCDP struct {
navigateCalled bool
navigateCalls int
navigateURL string
navigateErrorText string
navigateBlock time.Duration
pageStates []string
pageStateIndex int
}
Expand Down Expand Up @@ -115,7 +117,20 @@ func (f *fakeCDP) handler(w http.ResponseWriter, r *http.Request) {
var params map[string]any
_ = json.Unmarshal(req.Params, &params)
f.navigateURL, _ = params["url"].(string)
result = map[string]any{"frameId": "frame-1"}
// Page.navigate resolves on commit, so a slow origin holds the
// response open rather than replying and navigating later.
if f.navigateBlock > 0 {
select {
case <-time.After(f.navigateBlock):
case <-ctx.Done():
return
}
}
nav := map[string]any{"frameId": "frame-1"}
if f.navigateErrorText != "" {
nav["errorText"] = f.navigateErrorText
}
result = nav
case "Runtime.evaluate":
state := `{"url":"about:blank","readyState":"loading"}`
if len(f.pageStates) > 0 {
Expand Down Expand Up @@ -235,6 +250,54 @@ func TestSetDeviceMetricsOverride(t *testing.T) {
})
}

func TestDispatchStartURL(t *testing.T) {
t.Run("reports no error text on a committed navigation", func(t *testing.T) {
f := &fakeCDP{pageTargetID: "target-123", sessionID: "session-abc"}
url := startFakeCDP(t, f)

ctx, cancel := context.WithTimeout(context.Background(), time.Second)
defer cancel()
errorText, err := DispatchStartURL(ctx, url, "https://example.com/")
require.NoError(t, err)
assert.Empty(t, errorText)
assert.Equal(t, "https://example.com/", f.navigateURL)
})

// Chrome answers Page.navigate with an errorText and commits an error page
// when the requested URL cannot load, which must not read as success.
t.Run("surfaces the navigation error text", func(t *testing.T) {
f := &fakeCDP{
pageTargetID: "target-123",
sessionID: "session-abc",
navigateErrorText: "net::ERR_CONNECTION_REFUSED",
}
url := startFakeCDP(t, f)

ctx, cancel := context.WithTimeout(context.Background(), time.Second)
defer cancel()
errorText, err := DispatchStartURL(ctx, url, "https://unreachable.example/")
require.NoError(t, err)
assert.Equal(t, "net::ERR_CONNECTION_REFUSED", errorText)
})

// When the origin is slower to commit than the caller's budget, the caller
// has to see the failure rather than assume the page was replaced.
t.Run("errors when the navigation does not commit within the budget", func(t *testing.T) {
f := &fakeCDP{
pageTargetID: "target-123",
sessionID: "session-abc",
navigateBlock: 2 * time.Second,
}
url := startFakeCDP(t, f)

ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond)
defer cancel()
_, err := DispatchStartURL(ctx, url, "https://slow.example/")
require.Error(t, err)
assert.Contains(t, err.Error(), "Page.navigate")
})
}

func TestDispatchStartURLAndWait(t *testing.T) {
t.Run("waits for loaded destination", func(t *testing.T) {
f := &fakeCDP{
Expand Down
11 changes: 7 additions & 4 deletions server/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1253,9 +1253,12 @@ paths:
description: |
Optional multipart parts apply configuration while Chromium stays stopped once (policy,
flags, extensions, profile archive, optional display sizing), then Chromium is started exactly
once and DevTools readiness is awaited. Optional `start_url` dispatches a best-effort
navigation after readiness without waiting for page load. Bare hosts are normalized to
`https://`. Omit any part you do not need. At least one actionable part must be present.
once and DevTools readiness is awaited. Optional `start_url` is navigated after readiness and
awaited until the navigation commits, so the requested document has replaced whatever the
browser was showing; subresources may still be loading. A navigation that fails or exceeds
the internal budget is logged and still returns 200, leaving the browser on its
pre-navigation page. Bare hosts are normalized to `https://`. Omit any part you do not need.
At least one actionable part must be present.
Required configuration steps run in this order: policies, extensions, display, flags,
then profile archive. Chromium is started only once at the end. The endpoint is not
transactional: if a later required step fails, earlier successful side effects may remain
Expand Down Expand Up @@ -1315,7 +1318,7 @@ paths:
required: [zip_file, name]
responses:
"200":
description: Configuration applied; optional navigate completed successfully.
description: Configuration applied; optional navigate committed, failed, or timed out.
content:
application/json:
schema:
Expand Down
Loading