diff --git a/README.md b/README.md index 58c37aa..f25ad7e 100644 --- a/README.md +++ b/README.md @@ -253,7 +253,7 @@ A crawl is a conga job (`golem15.seo.crawl`, one attempt) started from **Start c **Raw HTML against the CMS.** The server-rendered HTML is checked against the record behind the page (`content_js` with `details.check: cms`): the record's title must appear in an H1, the text must reach half of the record's word count, and the record's primary parent (the breadcrumb link) must be linked. The comparison with the JavaScript-rendered page is `details.check: rendered` (next section); without a Chrome path every crawl records it as skipped (P6 and `seo_get_crawl` say so). -**Rendered comparison.** When `golem15.seo.crawl_chrome_path` names a Chrome or Chromium binary the operator installed on the server (none is installed by default), the crawler renders the first `golem15.seo.crawl_render_max` pages (default 200, in crawl order, so the home page and the top-level pages come first) in headless Chrome through `github.com/chromedp/chromedp`, right after each page's raw fetch, and compares the DOM after JavaScript with the raw HTML: same-host links, an H1, or at least 50 words and 20 % more text that appear only after JavaScript are one `content_js` finding with `details.check: rendered` and `details.what` naming them (up to five links). The browser is held to the crawler's limits: before Chrome starts, the site host and its `www` twin are resolved and every address checked like the crawler's dial guard (`fetchguard.IsPrivateAddr`; a private address refuses unless `golem15.seo.crawl_trusted` is on), and Chrome is pinned to those addresses through `--host-resolver-rules` while every other host resolves to nothing; every request is intercepted and continued only when it is https (http only in trusted mode) to the site host or its twin, or a `data:`/`blob:` URL, so third-party scripts, fonts, analytics and metadata addresses are never loaded. Each crawl gets a fresh temporary profile, removed when the crawl ends, with no cookies, credentials or extensions and downloads denied; one tab renders at a time, 20 s per page, the DOM is capped at 5 MiB, Chrome's own sandbox stays on, and Chrome is closed when the crawl finishes, fails or is cancelled. The path is config only (or `SUMMER_GOLEM15__SEO__CRAWL_CHROME_PATH`), never a setting or MCP value. The crawl records `render_status` `done` with the pages compared, `skipped` without a path, or `failed` with the reason (a relative path or one that is not an executable file, a private address, Chrome not starting, a page that cannot be rendered); a renderer failure never fails the crawl, and P6, `GET /_seo/api/v1/crawls/{id}` and `seo_get_crawl` (`render {status, pages, note, message}`) say which. +**Rendered comparison.** When `golem15.seo.crawl_chrome_path` names a Chrome or Chromium binary the operator installed on the server (none is installed by default), the crawler renders the first `golem15.seo.crawl_render_max` pages (default 200, in crawl order, so the home page and the top-level pages come first) in headless Chrome through `github.com/chromedp/chromedp`, right after each page's raw fetch, and compares the DOM after JavaScript with the raw HTML: same-host links, an H1, or at least 50 words and 20 % more text that appear only after JavaScript are one `content_js` finding with `details.check: rendered` and `details.what` naming them (up to five links). The browser is held to the crawler's limits: before Chrome starts, the site host and its `www` twin are resolved and every address checked like the crawler's dial guard (`fetchguard.IsPrivateAddr`; a private address refuses unless `golem15.seo.crawl_trusted` is on), and Chrome is pinned to those addresses through `--host-resolver-rules` while every other host resolves to nothing; every request is intercepted and continued only when it is https (http only in trusted mode) to the site host or its twin, or a `data:`/`blob:` URL, so third-party scripts, fonts, analytics and metadata addresses are never loaded. Each crawl gets a fresh temporary profile, removed when the crawl ends, with no cookies, credentials or extensions and downloads denied; one tab renders at a time, at most 5 s to open the tab (`crawl.TabOpenTimeout`) and 20 s for the page under one deadline, so a hung Chrome cannot stall the crawl, the DOM is capped at 5 MiB, Chrome's own sandbox stays on, and Chrome is closed when the crawl finishes, fails or is cancelled. The path is config only (or `SUMMER_GOLEM15__SEO__CRAWL_CHROME_PATH`), never a setting or MCP value. The crawl records `render_status` `done` with the pages compared, `skipped` without a path, or `failed` with the reason (a relative path or one that is not an executable file, a private address, Chrome not starting, a page that cannot be rendered); a renderer failure never fails the crawl, and P6, `GET /_seo/api/v1/crawls/{id}` and `seo_get_crawl` (`render {status, pages, note, message}`) say which. **Fix routes.** An issue a CMS value fixes names its tool: `seo_set_url_seo` (titles, descriptions, canonicals, sitemap membership of a record, structured data), `seo_update_robots`, `seo_create_redirect` (a missing page, a page that answers an error), `seo_create_page` (a page missing from the registry), `seo_update_settings` (publisher and site nodes), `sitemap_update_settings` (a listed noindex address without a CMS record, an Additional address of the sitemap: remove it there; `details.extra` is true, and without the sitemap plugin the issue is `fix: code` naming the sitemap's address list), or the content type's own tool from `pact.ContentDescriptor.Tools` when the problem sits in the record's content: H1 findings prefer `Tools[headings]`, image alt text `Tools[image_alt]`, other content findings `Tools[edit]` (the registry pages name `seo_update_page`). A finding of the site's template code, or of content whose type names no fitting tool, is `fix: code` with `details.what` naming the element to change. No issue has neither. diff --git a/classes/crawl/render_chrome.go b/classes/crawl/render_chrome.go index ff7ba5f..cfd7f79 100644 --- a/classes/crawl/render_chrome.go +++ b/classes/crawl/render_chrome.go @@ -7,8 +7,10 @@ import ( "net/netip" "os" "sort" + "strconv" "strings" "sync" + "time" "git.golem15.com/golem15/summercms/modules/fetchguard" "github.com/chromedp/cdproto/browser" @@ -31,8 +33,11 @@ import ( // BlockedByClient, so third-party scripts are never loaded; // - a fresh temporary profile per crawl, removed by Close; no cookies, // credentials or extensions; downloads denied; -// - one tab at a time (the crawler serialises Render), 20 s per page, the -// DOM capped at 5 MiB; Close stops Chrome. +// - one tab at a time (the crawler serialises Render), at most +// TabOpenTimeout (5 s) to open the tab and RenderTimeout (20 s) for the +// page, both under one deadline, so a hung Chrome never holds the +// crawl's render lock longer; the DOM capped at 5 MiB; Close stops +// Chrome. // // chromedp's default allocator flags are used as they are: nothing is // added that weakens Chrome's sandbox. @@ -45,11 +50,30 @@ type chromeRenderer struct { browserCtx context.Context browserCancel context.CancelFunc + // newTab and startTab open a tab: chromedp.NewContext and chromedp.Do + // unless a test replaces them. + newTab func(parent context.Context) (context.Context, context.CancelFunc) + startTab func(tab context.Context) error + mu sync.Mutex blocked []string closed bool } +// TabOpenTimeout bounds opening a Chrome tab (P33-60, L3 WR-06); Render +// bounds opening the tab and rendering the page with one deadline of +// TabOpenTimeout plus RenderTimeout. +var TabOpenTimeout = 5 * time.Second + +// seconds writes d as whole seconds ("5 s") when it is one, else as +// time.Duration does. +func seconds(d time.Duration) string { + if d%time.Second == 0 { + return strconv.Itoa(int(d/time.Second)) + " s" + } + return d.String() +} + // newChromeRenderer starts Chrome at path for the hosts and their checked // addresses. func newChromeRenderer(path string, hosts map[string][]netip.Addr, trusted bool, ua string) (*chromeRenderer, error) { @@ -96,14 +120,42 @@ func (r *chromeRenderer) Render(ctx context.Context, pageURL string) (string, er if !allowRequest(pageURL, r.allowed, r.trusted) { return "", fmt.Errorf("the address %s is outside the site", pageURL) } - tab, closeTab := chromedp.NewContext(r.browserCtx) + // One deadline bounds opening the tab and rendering the page; the tab + // ends with it or with the crawl's context (a cancelled crawl). + rctx, rcancel := context.WithTimeout(ctx, TabOpenTimeout+RenderTimeout) + defer rcancel() + newTab, startTab := r.newTab, r.startTab + if newTab == nil { + newTab = func(parent context.Context) (context.Context, context.CancelFunc) { return chromedp.NewContext(parent) } + } + if startTab == nil { + startTab = func(tab context.Context) error { return chromedp.Do(tab) } + } + tab, closeTab := newTab(r.browserCtx) defer closeTab() - // The tab ends with the crawl's context (a cancelled crawl) or after - // the page's timeout. - stop := context.AfterFunc(ctx, closeTab) + stop := context.AfterFunc(rctx, closeTab) defer stop() - if err := chromedp.Do(tab); err != nil { - return "", fmt.Errorf("Chrome cannot open a tab: %w", err) + opened := make(chan error, 1) + go func() { opened <- startTab(tab) }() + openTimer := time.NewTimer(TabOpenTimeout) + defer openTimer.Stop() + select { + case err := <-opened: + if err != nil { + if ctx.Err() != nil { + return "", ctx.Err() + } + return "", fmt.Errorf("Chrome cannot open a tab: %w", err) + } + case <-openTimer.C: + closeTab() + return "", fmt.Errorf("Chrome did not open a tab within %s", seconds(TabOpenTimeout)) + case <-rctx.Done(): + closeTab() + if ctx.Err() != nil { + return "", ctx.Err() + } + return "", fmt.Errorf("Chrome did not open a tab within %s", seconds(TabOpenTimeout)) } page, cancel := context.WithTimeout(tab, RenderTimeout) defer cancel() diff --git a/classes/crawl/render_test.go b/classes/crawl/render_test.go index a8a76f2..cb8c146 100644 --- a/classes/crawl/render_test.go +++ b/classes/crawl/render_test.go @@ -8,6 +8,7 @@ import ( "path/filepath" "strings" "testing" + "time" "git.golem15.com/golem15/summercms/modules/compass" ) @@ -124,3 +125,42 @@ func TestRenderMax(t *testing.T) { t.Fatalf("SiteHosts = %v", got) } } + +// TestRenderTabTimeout pins L3 WR-06: a tab that does not open answers an +// error within TabOpenTimeout (lowered for the test) plus a margin, so the +// crawl's render lock is released; the next Render is not held up by the +// first one's hung tab. +func TestRenderTabTimeout(t *testing.T) { + saved := TabOpenTimeout + TabOpenTimeout = 50 * time.Millisecond + t.Cleanup(func() { TabOpenTimeout = saved }) + hung := make(chan struct{}) + t.Cleanup(func() { close(hung) }) + r := &chromeRenderer{ + allowed: []string{"example.test"}, + browserCtx: context.Background(), + newTab: func(parent context.Context) (context.Context, context.CancelFunc) { + return context.WithCancel(parent) + }, + // A hung Chrome: opening the tab never answers, even when the tab + // is closed. + startTab: func(context.Context) error { + <-hung + return nil + }, + } + for i := range 2 { + start := time.Now() + _, err := r.Render(t.Context(), "https://example.test/a") + if err == nil || !strings.Contains(err.Error(), "did not open a tab within 50ms") { + t.Fatalf("Render %d: %v, want the tab timeout", i, err) + } + if d := time.Since(start); d > time.Second { + t.Fatalf("Render %d returned after %v", i, d) + } + } + TabOpenTimeout = 5 * time.Second + if got := seconds(TabOpenTimeout); got != "5 s" { + t.Fatalf("seconds = %q", got) + } +}