diff --git a/README.md b/README.md index a3c0614..58c37aa 100644 --- a/README.md +++ b/README.md @@ -247,7 +247,7 @@ A crawl is a conga job (`golem15.seo.crawl`, one attempt) started from **Start c **What it fetches.** `robots.txt`, the sitemap index at `/sitemap.xml` and its files (or a single urlset), then breadth-first from the home page and the sitemap addresses over same-host links, redirect targets, canonicals and hreflang alternates. Requests go through a `fetchguard` client limited to the site host and its `www` twin: HTTPS only, private and reserved addresses refused at dial time, redirects never followed (each `Location` is a new address checked again), the User-Agent `SummerCMS-SEO-Crawler/1 (+)`. Links to other hosts, credentials in a URL and plain http are never fetched. A staging or local site served over http or on a private address needs `golem15.seo.crawl_trusted: true` in config (or `SUMMER_GOLEM15__SEO__CRAWL_TRUSTED`), which no setting or MCP call can set; trusted mode still fetches only the site host and its twin. robots.txt rules are read for the report, not obeyed. -**Limits.** Two requests at a time with the settings' pause between requests (default 250 ms), a 10 s timeout and a 5 MiB body cap per request, at most 1000 links read per page, and the settings' page limit (default 5000; a crawl that stops there is marked "limit reached"). Cancelling (**Cancel crawl**, `seo_cancel_crawl`) stops between requests and keeps what was fetched; a failed crawl keeps the pages and issues found before the failure and records the error. A crawl that reaches the job's time limit (`crawl.JobTimeout`, 6 h) ends failed with "the crawl reached its time limit of 6h" and keeps what it fetched: the final writes run on a context the job's cancellation cannot reach, bounded by `crawl.FinishTimeout` (10 min). The crawler writes only its own tables (`golem15_seo_crawls`, `golem15_seo_crawl_pages`, `golem15_seo_crawl_links`, `golem15_seo_issues`); crawls beyond the settings' "Crawls kept" are deleted with their pages, links and issues. +**Limits.** Two requests at a time with the settings' pause between requests (default 250 ms), a 10 s timeout and a 5 MiB body cap per request, at most 1000 links read per page, and the settings' page limit (default 5000; a crawl that stops there is marked "limit reached"). Cancelling (**Cancel crawl**, `seo_cancel_crawl`) ends a queued crawl at once; a running crawl records the request (`cancel_requested` in the API and MCP answers, **Cancelling** in the list) and keeps running until its job stops between requests, then ends cancelled with what was fetched. Until then no other crawl starts (409 `crawls.already_running`) and the crawl cannot be deleted; a failed crawl keeps the pages and issues found before the failure and records the error. A crawl that reaches the job's time limit (`crawl.JobTimeout`, 6 h) ends failed with "the crawl reached its time limit of 6h" and keeps what it fetched: the final writes run on a context the job's cancellation cannot reach, bounded by `crawl.FinishTimeout` (10 min). A running crawl's job writes `heartbeat_at` every minute (`crawl.HeartbeatEvery`); a running crawl without a heartbeat for 10 minutes (`crawl.StaleAfter`, its job is gone, for example after a worker restart) is ended by the next start or delete (`crawl.RecoverStale`): failed with "the crawl job stopped without finishing (no heartbeat since …)", or cancelled when its cancel was requested. The crawler writes only its own tables (`golem15_seo_crawls`, `golem15_seo_crawl_pages`, `golem15_seo_crawl_links`, `golem15_seo_issues`); crawls beyond the settings' "Crawls kept" are deleted with their pages, links and issues. **Checks** (kind, severity): `page_status` (error), `sitemap_error` (error), `sitemap_noindex` (error), `sitemap_blocked` (error), `sitemap_missing` (notice), `broken_link` (error, with the linking pages), `redirected_link` (notice), `canonical_missing` (warning), `canonical_error` (error), `missing_title` and `duplicate_title` (warning), `missing_description` and `duplicate_description` (notice), `h1_missing` and `h1_multiple` (error), `heading_skipped` and `heading_empty` (warning), `h1_title_mismatch` (notice), `jsonld_invalid` (error), `jsonld_multiple` (warning), `jsonld_required` (error) and `jsonld_recommended` (notice) from the same rules as the structured data preview (`schema.Validate`; FAQPage and other types without Google rules find nothing), `hreflang_reciprocal` (warning), `hreflang_error` (error), `generic_anchor` (warning, the settings' generic link texts, whole text, any case), `image_alt` (warning: an `` without `alt`; `alt=""` is decorative), `image_size` and `featured_lazy` (notice), `content_js` (warning) and `orphan_page` (warning: an indexable sitemap page no crawled page links to, with up to 10 pages whose text mentions its title or focus keyphrase). The CMS-data checks run at the end of every crawl (and when the list is opened before any crawl) and are stored with the crawl's issues as source `cms`; a CMS issue the crawl found too is stored once. diff --git a/classes/crawl/actions.go b/classes/crawl/actions.go index fa6e74d..e0e430b 100644 --- a/classes/crawl/actions.go +++ b/classes/crawl/actions.go @@ -71,11 +71,16 @@ func Unignore(tx *gorm.DB, issues []models.Issue) (int, error) { } // DeleteCrawls deletes crawls with their pages, links and issues; a -// queued or running one refuses the whole call with ErrDeleteRunning. +// queued or running one (also one whose cancel is requested) refuses the +// whole call with ErrDeleteRunning. A running crawl without a heartbeat is +// ended first (RecoverStale), so it can be deleted. func DeleteCrawls(tx *gorm.DB, ids []uint) (int, error) { if len(ids) == 0 { return 0, nil } + if _, err := RecoverStale(tx, time.Now()); err != nil { + return 0, err + } var active int64 if err := tx.Model(&models.Crawl{}).Where("id IN ? AND status IN ?", ids, []string{models.CrawlQueued, models.CrawlRunning}).Count(&active).Error; err != nil { return 0, err diff --git a/classes/crawl/job.go b/classes/crawl/job.go index b37da2a..510766e 100644 --- a/classes/crawl/job.go +++ b/classes/crawl/job.go @@ -45,12 +45,19 @@ func (CrawlArgs) Kind() string { return CrawlKind } // The crawl job's time bounds (P33-60). JobTimeout is the job's deadline; // FinishTimeout bounds storing what a crawl found, on a context the job's -// cancellation cannot reach. +// cancellation cannot reach; the job writes heartbeat_at every +// HeartbeatEvery, and a running crawl without a heartbeat for StaleAfter +// is ended by the next Start or delete (RecoverStale). const ( - JobTimeout = 6 * time.Hour - FinishTimeout = 10 * time.Minute + JobTimeout = 6 * time.Hour + FinishTimeout = 10 * time.Minute + HeartbeatEvery = time.Minute + StaleAfter = 10 * time.Minute ) +// heartbeatWrite bounds one heartbeat update. +const heartbeatWrite = 10 * time.Second + // The crawl limits used until the Site audit settings say otherwise. const ( DefaultMaxPages = 5000 @@ -78,9 +85,11 @@ func (e *AlreadyRunningError) Since() time.Time { // Start queues a crawl (D-14, P33-39) inside tx: the settings row is // locked so two starts cannot both pass the check that no crawl is queued -// or running, then the crawl row is written and the conga job dispatched -// in the same transaction. A missing site address refuses with -// ErrNoSiteURL, a running crawl with *AlreadyRunningError. +// or running, a running crawl whose job stopped writing its heartbeat is +// ended (RecoverStale), then the crawl row is written and the conga job +// dispatched in the same transaction. A missing site address refuses with +// ErrNoSiteURL, a queued or running crawl (also one whose cancel is +// requested but whose job has not stopped) with *AlreadyRunningError. func Start(ctx context.Context, svc *classes.Service, tx *gorm.DB, trigger, name string) (models.Crawl, error) { site := svc.SiteURL() if site == "" { @@ -92,6 +101,11 @@ func Start(ctx context.Context, svc *classes.Service, tx *gorm.DB, trigger, name if err := tx.Exec(`SELECT id FROM golem15_seo_settings WHERE id = 1 FOR UPDATE`).Error; err != nil { return models.Crawl{}, err } + stale, err := RecoverStale(tx, time.Now().UTC()) + if err != nil { + return models.Crawl{}, err + } + endStaleJobs(ctx, svc, tx, stale) active, err := ActiveCrawl(tx) if err != nil { return models.Crawl{}, err @@ -182,6 +196,7 @@ func LoadCrawl(db *gorm.DB, id uint) (models.Crawl, error) { // context) or at the job timeout; what was fetched is then stored on a // context detached from it with its own FinishTimeout bound, so a // cancelled or timed-out crawl keeps its results and never stays running. +// While it runs, the job writes heartbeat_at every HeartbeatEvery. func Run(ctx context.Context, svc *classes.Service, id uint) error { gdb, err := svc.GormDB() if err != nil { @@ -197,19 +212,49 @@ func Run(ctx context.Context, svc *classes.Service, id uint) error { } now := time.Now().UTC() moved := db.Model(&models.Crawl{}).Where("id = ? AND status = ?", id, models.CrawlQueued). - Updates(map[string]any{"status": models.CrawlRunning, "started_at": now}) + Updates(map[string]any{"status": models.CrawlRunning, "started_at": now, "heartbeat_at": now}) if moved.Error != nil { return moved.Error } if moved.RowsAffected == 0 { return nil } + stop := heartbeat(ctx, gdb, id) + defer stop() res, runErr := run(ctx, svc, db, row) fctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), FinishTimeout) defer cancel() return finish(fctx, svc, gdb.WithContext(fctx), row, res, runErr, ctx.Err()) } +// heartbeat writes the crawl's heartbeat_at every HeartbeatEvery until the +// returned stop is called; each write runs on a context detached from the +// job's, bounded by heartbeatWrite. +func heartbeat(ctx context.Context, gdb *gorm.DB, id uint) (stop func()) { + done := make(chan struct{}) + finished := make(chan struct{}) + go func() { + defer close(finished) + t := time.NewTicker(HeartbeatEvery) + defer t.Stop() + for { + select { + case <-done: + return + case <-t.C: + hctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), heartbeatWrite) + _ = gdb.WithContext(hctx).Model(&models.Crawl{}).Where("id = ? AND status = ?", id, models.CrawlRunning). + Update("heartbeat_at", time.Now().UTC()).Error + cancel() + } + } + }() + return func() { + close(done) + <-finished + } +} + // run fetches and checks the site of one crawl. func run(ctx context.Context, svc *classes.Service, db *gorm.DB, row models.Crawl) (*Result, error) { f, err := NewFetcher(row.Site, svc.Config()) @@ -267,14 +312,17 @@ func run(ctx context.Context, svc *classes.Service, db *gorm.DB, row models.Craw return Crawl(ctx, f, opt) } -// cancelRequested reports whether the crawl was cancelled: its status is -// cancelled. +// cancelRequested reports whether a cancel of the crawl was requested: its +// cancel_requested_at is set, or its status is cancelled. func cancelRequested(db *gorm.DB, id uint) bool { - var status string - if err := db.Raw(`SELECT status FROM golem15_seo_crawls WHERE id = ?`, id).Scan(&status).Error; err != nil { + var rows []struct { + Status string + Requested bool + } + if err := db.Raw(`SELECT status, cancel_requested_at IS NOT NULL AS requested FROM golem15_seo_crawls WHERE id = ?`, id).Scan(&rows).Error; err != nil || len(rows) == 0 { return false } - return status == models.CrawlCancelled + return rows[0].Requested || rows[0].Status == models.CrawlCancelled } // timeLimitError is the error of a crawl that reached JobTimeout. @@ -370,6 +418,31 @@ func finish(ctx context.Context, svc *classes.Service, db *gorm.DB, row models.C return nil } +// endStaleJobs fails or stops the conga jobs of crawls RecoverStale ended, +// once tx commits; best effort. +func endStaleJobs(ctx context.Context, svc *classes.Service, tx *gorm.DB, rows []models.Crawl) { + if len(rows) == 0 { + return + } + lagoon.AfterCommit(ctx, tx, func(ctx context.Context, _ *gorm.DB) { + m, err := conga.From(svc.App()) + if err != nil { + return + } + for _, r := range rows { + if r.JobID == nil { + continue + } + meta := map[string]any{"crawl_id": r.ID, "status": r.Status} + if r.Status == models.CrawlCancelled { + _ = m.StopJob(ctx, *r.JobID, meta) + } else { + _ = m.FailJob(ctx, *r.JobID, meta) + } + } + }) +} + // Lookup returns the CMS record behind a crawled address (cached per // address): the content type whose Match answers the path exactly, its // descriptor facts in the matched locale, its primary parent's address and @@ -455,20 +528,31 @@ func Keyphrases(db *gorm.DB) (map[string]string, error) { // running. var errCancelledCrawl = errors.New("golem15.seo: the crawl is not running") -// Cancel stops a queued or running crawl inside tx: the row is marked -// cancelled by name (the crawl keeps what it fetched) and its conga job -// cancelled. +// Cancel stops a queued or running crawl inside tx. A queued crawl ends +// cancelled at once (by name, finished_at set). A running crawl gets +// cancel_requested_at and cancelled_by and stays running: its job stops +// between requests and writes cancelled with the pages and issues fetched +// so far, and until then no other crawl starts. The conga job is cancelled +// in both cases. func Cancel(ctx context.Context, svc *classes.Service, tx *gorm.DB, row models.Crawl, name string) error { if !row.Active() { return errCancelledCrawl } now := time.Now().UTC() - updates := map[string]any{"status": models.CrawlCancelled, "cancelled_by": name} - if row.Status == models.CrawlQueued { - updates["finished_at"] = now + queued := tx.Model(&models.Crawl{}).Where("id = ? AND status = ?", row.ID, models.CrawlQueued). + Updates(map[string]any{"status": models.CrawlCancelled, "cancelled_by": name, "finished_at": now}) + if queued.Error != nil { + return queued.Error } - if err := tx.Model(&models.Crawl{}).Where("id = ?", row.ID).Updates(updates).Error; err != nil { - return err + if queued.RowsAffected == 0 { + running := tx.Model(&models.Crawl{}).Where("id = ? AND status = ?", row.ID, models.CrawlRunning). + Updates(map[string]any{"cancel_requested_at": now, "cancelled_by": name}) + if running.Error != nil { + return running.Error + } + if running.RowsAffected == 0 { + return errCancelledCrawl + } } if row.JobID != nil { if m, err := conga.From(svc.App()); err == nil { diff --git a/classes/crawl/store.go b/classes/crawl/store.go index da7e66a..54a0507 100644 --- a/classes/crawl/store.go +++ b/classes/crawl/store.go @@ -189,6 +189,55 @@ func ActiveCrawl(db *gorm.DB) (*models.Crawl, error) { return &rows[0], nil } +// RecoverStale ends the running crawls whose job stopped writing its +// heartbeat (P33-60): a running row whose COALESCE(heartbeat_at, +// started_at, created_at) is older than StaleAfter before now becomes +// cancelled when its cancel was requested, else failed with the error +// "the crawl job stopped without finishing (no heartbeat since