diff --git a/.mdox.validate.yaml b/.mdox.validate.yaml index 98d111c..e7d6ab2 100644 --- a/.mdox.validate.yaml +++ b/.mdox.validate.yaml @@ -3,4 +3,8 @@ version: 1 validators: # Validators to skip checking PR/issue links of mdox. - regex: '(^http[s]?:\/\/)(www\.)?(github\.com\/)bwplotka\/mdox(\/pull\/|\/issues\/)' - type: 'githubPullsIssues' \ No newline at end of file + type: 'githubPullsIssues' + + # Skip the Slack channel link to avoid requests that require authentication. + - regex: '^https://cloud-native\.slack\.com/archives/mdox$' + type: 'ignore' diff --git a/README.md b/README.md index 9ce2fad..20e31a3 100644 --- a/README.md +++ b/README.md @@ -150,7 +150,7 @@ validators: As seen above, mdox supports validate configuration supports a few parameters and passing an array of link validators with types and regexes. The supported configuration parameters are: -* `timeout`: The HTTP client's timeout. Defaults to "10s". +* `timeout`: The HTTP client's timeout. When configured, it also sets the response header timeout. Defaults to "10s". * `parallelism`: The maximum amount of concurrent HTTP requests. Defaults to 100. * `host_max_conns`: The maximum amount of HTTP connections open per host. Defaults to 2. * `random_delay`: A random delay between 0 and this value is added between requests. It takes values like "500ms", "1s", "1m", or "1m30s". Defaults to no delay. @@ -159,7 +159,7 @@ There are three types of validators: * `ignore`: This type of validator makes sure that `mdox` does not check links with provided regex. This is the most common use case. * `githubPullsIssues`: This is a smart validator which only accepts a specific type of regex of the form `(^http[s]?:\/\/)(www\.)?(github\.com\/){ORG}\/{REPO}(\/pull\/|\/issues\/)`. It performs smart validation on GitHub PR and issues links, by fetching GitHub API to get the latest pull/issue number and matching regex. This makes sure that mdox doesn't get rate limited by GitHub, even when checking a large number of GitHub links(which is pretty common in documentation)! -* `roundtrip`: All links are checked with the roundtrip validator by default(no need for including into config explicitly) which means that each link is visited and fails if http status code is not 200(even after retries). +* `roundtrip`: All links are checked with the roundtrip validator by default(no need for including into config explicitly). Transport errors and HTTP 429, 500, 502, 503, and 504 responses are retried up to three times, with backoff delays of 2, 4, and 8 seconds. A valid `Retry-After` header (seconds or an HTTP date) can extend the delay up to one minute. Longer requested delays are reported as errors without retrying. Redirects are followed by the HTTP client. Links that still fail after retries are reported as errors. Rate limiting does not count as successful validation. Relative link checking *is not* affected by this configuration, as it is expected that such links will work. diff --git a/pkg/mdformatter/linktransformer/link.go b/pkg/mdformatter/linktransformer/link.go index 86b9b0c..c28e0cb 100644 --- a/pkg/mdformatter/linktransformer/link.go +++ b/pkg/mdformatter/linktransformer/link.go @@ -12,7 +12,6 @@ import ( "io" "net" "net/http" - "net/url" "os" "path/filepath" "regexp" @@ -99,6 +98,9 @@ func newLinktransformerMetrics(reg *prometheus.Registry) *linktransformerMetrics const ( originalURLKey = "originalURLKey" numberOfRetriesKey = "retryKey" + + maxRetries = 3 + maxRetryDelay = time.Minute ) type chain struct { @@ -189,8 +191,7 @@ type validator struct { futureMu sync.Mutex destFutures map[futureKey]*futureResult - l *linktransformerMetrics - transportFn func(url string) http.RoundTripper + l *linktransformerMetrics } type futureKey struct { @@ -203,19 +204,7 @@ type futureResult struct { cases int } -// NewValidator returns mdformatter.LinkTransformer that crawls all links. -// TODO(bwplotka): Add optimization and debug modes - this is the main source of latency and pain. -func NewValidator(ctx context.Context, logger log.Logger, linksValidateConfig []byte, anchorDir string, storage *cache.SQLite3Storage, reg *prometheus.Registry) (mdformatter.LinkTransformer, error) { - var err error - config := Config{} - if string(linksValidateConfig) != "" { - config, err = ParseConfig(linksValidateConfig) - if err != nil { - return nil, err - } - } - - linktransformerMetrics := newLinktransformerMetrics(reg) +func newHTTPTransport(config Config) *http.Transport { transport := &http.Transport{ Proxy: http.ProxyFromEnvironment, ForceAttemptHTTP2: true, @@ -232,6 +221,26 @@ func NewValidator(ctx context.Context, logger log.Logger, linksValidateConfig [] if config.HostMaxConns != nil { transport.MaxConnsPerHost = *config.HostMaxConns } + if config.Timeout != "" { + transport.ResponseHeaderTimeout = config.timeout + } + return transport +} + +// NewValidator returns mdformatter.LinkTransformer that crawls all links. +// TODO(bwplotka): Add optimization and debug modes - this is the main source of latency and pain. +func NewValidator(ctx context.Context, logger log.Logger, linksValidateConfig []byte, anchorDir string, storage *cache.SQLite3Storage, reg *prometheus.Registry) (mdformatter.LinkTransformer, error) { + var err error + config := Config{} + if string(linksValidateConfig) != "" { + config, err = ParseConfig(linksValidateConfig) + if err != nil { + return nil, err + } + } + + linktransformerMetrics := newLinktransformerMetrics(reg) + transport := newHTTPTransport(config) v := &validator{ logger: logger, anchorDir: anchorDir, @@ -242,27 +251,25 @@ func NewValidator(ctx context.Context, logger log.Logger, linksValidateConfig [] storage: nil, destFutures: map[futureKey]*futureResult{}, l: linktransformerMetrics, - transportFn: func(u string) http.RoundTripper { - parsed, err := url.Parse(u) - if err != nil { - panic(err) - } - return promhttp.InstrumentRoundTripperCounter( - linktransformerMetrics.collyRequests, - promhttp.InstrumentRoundTripperDuration( - linktransformerMetrics.collyPerDomainLatency.MustCurryWith(prometheus.Labels{"domain": parsed.Host}), - transport, - ), - ) - }, - } - - // Set very soft limits. - // E.g GitHub has 50-5000 https://docs.github.com/en/free-pro-team@latest/rest/reference/rate-limit limit depending - // on API (only search is below 100). + } + if config.Timeout != "" { v.c.SetRequestTimeout(config.timeout) } + // Install the transport once to avoid races between async requests. + v.c.WithTransport(promhttp.InstrumentRoundTripperCounter( + linktransformerMetrics.collyRequests, + // Observe duration directly instead of using promhttp.InstrumentRoundTripperDuration + // because the domain label depends on each request's URL host. + promhttp.RoundTripperFunc(func(r *http.Request) (*http.Response, error) { + start := time.Now() + resp, err := transport.RoundTrip(r) + if err == nil { + linktransformerMetrics.collyPerDomainLatency.WithLabelValues(r.URL.Host).Observe(time.Since(start).Seconds()) + } + return resp, err + }), + )) if v.validateConfig.Cache.IsSet() && storage != nil { v.storage = storage @@ -300,53 +307,96 @@ func NewValidator(ctx context.Context, logger log.Logger, linksValidateConfig [] v.remoteLinks[response.Ctx.Get(originalURLKey)] = nil }) v.c.OnError(func(response *colly.Response, err error) { - v.rMu.Lock() - defer v.rMu.Unlock() - retriesStr := response.Ctx.Get(numberOfRetriesKey) - retries, _ := strconv.Atoi(retriesStr) - switch response.StatusCode { - case http.StatusTooManyRequests: - if retries > 0 { - break - } - var retryAfterSeconds int - // Retry calls same methods as Visit and makes request with same options. - // So retryKey is incremented here if onError is called again after Retry. By default retries once. - response.Ctx.Put(numberOfRetriesKey, strconv.Itoa(retries+1)) - retryAfterSeconds, convErr := strconv.Atoi(response.Headers.Get("Retry-After")) - if convErr != nil { - retryAfterSeconds = 1 - } - select { - case <-time.After(time.Duration(retryAfterSeconds) * time.Second): - case <-v.c.Context.Done(): - return - } + shouldRetry, backoff := v.recordError(response, err) + if !shouldRetry { + return + } - if retryErr := response.Request.Retry(); retryErr != nil { - v.remoteLinks[response.Ctx.Get(originalURLKey)] = fmt.Errorf("remote link retry %v: %w", response.Ctx.Get(originalURLKey), err) - break - } - v.remoteLinks[response.Ctx.Get(originalURLKey)] = fmt.Errorf("%q rate limited even after retry; status code %v: %w", response.Request.URL.String(), response.StatusCode, err) - // 0 StatusCode means error on call side. - case http.StatusMovedPermanently, http.StatusTemporaryRedirect, http.StatusServiceUnavailable, 0: - if retries > 0 { - break - } - response.Ctx.Put(numberOfRetriesKey, strconv.Itoa(retries+1)) + // Stop canceled timers explicitly for Go versions before 1.23. + timer := time.NewTimer(backoff) + defer timer.Stop() + select { + case <-timer.C: + case <-v.c.Context.Done(): + } + if ctxErr := v.c.Context.Err(); ctxErr != nil { + v.rMu.Lock() + v.remoteLinks[response.Ctx.Get(originalURLKey)] = fmt.Errorf( + "%q not accessible, status code %v, canceled during backoff: %w", + response.Request.URL.String(), response.StatusCode, ctxErr) + v.rMu.Unlock() + return + } - if retryErr := response.Request.Retry(); retryErr != nil { - v.remoteLinks[response.Ctx.Get(originalURLKey)] = fmt.Errorf("remote link retry %v: %w", response.Ctx.Get(originalURLKey), err) - break - } - v.remoteLinks[response.Ctx.Get(originalURLKey)] = fmt.Errorf("%q not accessible even after retry; status code %v: %w", response.Request.URL.String(), response.StatusCode, err) - default: - v.remoteLinks[response.Ctx.Get(originalURLKey)] = fmt.Errorf("%q not accessible; status code %v: %w", response.Request.URL.String(), response.StatusCode, err) + if retryErr := response.Request.Retry(); retryErr != nil { + v.rMu.Lock() + v.remoteLinks[response.Ctx.Get(originalURLKey)] = fmt.Errorf("remote link retry %v: %w", response.Ctx.Get(originalURLKey), retryErr) + v.rMu.Unlock() } }) return v, nil } +// recordError evaluates a failed request and decides whether to retry. +// It returns (true, backoff) when a retry is warranted. +// Map writes happen under rMu so the caller can sleep without holding the lock. +func (v *validator) recordError(response *colly.Response, err error) (bool, time.Duration) { + v.rMu.Lock() + defer v.rMu.Unlock() + + originalURL := response.Ctx.Get(originalURLKey) + if ctxErr := v.c.Context.Err(); ctxErr != nil { + v.remoteLinks[originalURL] = fmt.Errorf("%q not accessible, status code %v: %w", response.Request.URL.String(), response.StatusCode, ctxErr) + return false, 0 + } + retries, _ := strconv.Atoi(response.Ctx.Get(numberOfRetriesKey)) + + switch response.StatusCode { + // Status 0 represents a transport error. HTTP redirects are followed by the client. + case 0, http.StatusTooManyRequests, http.StatusInternalServerError, + http.StatusBadGateway, http.StatusServiceUnavailable, http.StatusGatewayTimeout: + if retries < maxRetries { + backoff := time.Duration(1< maxRetryDelay { + return 0, fmt.Errorf("Retry-After %q exceeds maximum delay %v", retryAfter, maxRetryDelay) + } + if delay > backoff { + return delay, nil + } + return backoff, nil +} + // MustNewValidator returns mdformatter.LinkTransformer that crawls all links. func MustNewValidator(logger log.Logger, linksValidateConfig []byte, anchorDir string, storage *cache.SQLite3Storage) mdformatter.LinkTransformer { v, err := NewValidator(context.TODO(), logger, linksValidateConfig, anchorDir, storage, nil) diff --git a/pkg/mdformatter/linktransformer/link_test.go b/pkg/mdformatter/linktransformer/link_test.go index baea35a..5c4b5d3 100644 --- a/pkg/mdformatter/linktransformer/link_test.go +++ b/pkg/mdformatter/linktransformer/link_test.go @@ -381,7 +381,7 @@ validators: MustNewValidator(logger, []byte(""), anchorDir, nil), )) testutil.NotOk(t, err) - testutil.Assert(t, strings.Contains(err.Error(), fmt.Sprintf("%v:1: \"https://docs.gfoogle.com/drawings/d/e/2PACX-1vTBFK_cGMbxFpYcv/pub?w=960&h=720\" not accessible even after retry; status code 0", relDirPath+filePath))) + testutil.Assert(t, strings.Contains(err.Error(), fmt.Sprintf("%v:1: \"https://docs.gfoogle.com/drawings/d/e/2PACX-1vTBFK_cGMbxFpYcv/pub?w=960&h=720\" not accessible, status code 0 after %d retries", relDirPath+filePath, maxRetries))) testutil.Assert(t, strings.Contains(err.Error(), fmt.Sprintf("%v:1: \"https://bwplotka.dev/does-not-exists\" not accessible; status code 404: Not Found", relDirPath+filePath))) }) diff --git a/pkg/mdformatter/linktransformer/retry_test.go b/pkg/mdformatter/linktransformer/retry_test.go new file mode 100644 index 0000000..55bc1e9 --- /dev/null +++ b/pkg/mdformatter/linktransformer/retry_test.go @@ -0,0 +1,364 @@ +// Copyright (c) Bartłomiej Płotka @bwplotka +// Licensed under the Apache License 2.0. + +package linktransformer + +import ( + "context" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "regexp" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/bwplotka/mdox/pkg/mdformatter" + "github.com/efficientgo/core/testutil" + "github.com/go-kit/log" + "github.com/gocolly/colly/v2" + "github.com/prometheus/client_golang/prometheus" +) + +func remoteLinksFile(t *testing.T, links ...string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "links.md") + var doc []string + for _, link := range links { + doc = append(doc, "[link]("+link+")") + } + testutil.Ok(t, os.WriteFile(path, []byte(strings.Join(doc, "\n\n")+"\n"), 0600)) + return path +} + +func checkRemoteLinks(t *testing.T, ctx context.Context, v mdformatter.LinkTransformer, links ...string) error { + t.Helper() + path := remoteLinksFile(t, links...) + _, err := mdformatter.IsFormatted(ctx, log.NewNopLogger(), []string{path}, mdformatter.WithLinkTransformer(v)) + return err +} + +func TestValidator_RetryBehavior(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + firstStatus int + finalStatus int + wantRequests int32 + wantError string + }{ + {name: "429 recovers", firstStatus: 429, finalStatus: 200, wantRequests: 2}, + {name: "429 does not hide 404", firstStatus: 429, finalStatus: 404, wantRequests: 2, wantError: "status code 404"}, + {name: "429 exhausts retries", firstStatus: 429, finalStatus: 429, wantRequests: 4, wantError: "status code 429 after 3 retries"}, + {name: "500 recovers", firstStatus: 500, finalStatus: 200, wantRequests: 2}, + {name: "502 recovers", firstStatus: 502, finalStatus: 200, wantRequests: 2}, + {name: "503 recovers", firstStatus: 503, finalStatus: 200, wantRequests: 2}, + {name: "504 recovers", firstStatus: 504, finalStatus: 200, wantRequests: 2}, + {name: "503 exhausts retries", firstStatus: 503, finalStatus: 503, wantRequests: 4, wantError: "status code 503 after 3 retries"}, + {name: "EOF recovers", firstStatus: 0, finalStatus: 200, wantRequests: 2}, + {name: "404 is not retried", firstStatus: 404, finalStatus: 200, wantRequests: 1, wantError: "status code 404"}, + {name: "301 follows redirect", firstStatus: 301, finalStatus: 200, wantRequests: 2}, + {name: "307 follows redirect", firstStatus: 307, finalStatus: 200, wantRequests: 2}, + } { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + var requests int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + status := tc.finalStatus + if atomic.AddInt32(&requests, 1) == 1 { + status = tc.firstStatus + } + if status == 0 { + conn, _, err := w.(http.Hijacker).Hijack() + if err != nil { + t.Errorf("hijack connection: %v", err) + return + } + if err := conn.Close(); err != nil { + t.Errorf("close connection: %v", err) + } + return + } + if status == http.StatusMovedPermanently || status == http.StatusTemporaryRedirect { + http.Redirect(w, r, "/target", status) + return + } + w.WriteHeader(status) + })) + defer srv.Close() + + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + reg := prometheus.NewRegistry() + v, err := NewValidator(ctx, log.NewNopLogger(), nil, t.TempDir(), nil, reg) + testutil.Ok(t, err) + err = checkRemoteLinks(t, ctx, v, srv.URL) + if tc.wantError == "" { + testutil.Ok(t, err) + } else { + testutil.NotOk(t, err) + testutil.Assert(t, strings.Contains(err.Error(), tc.wantError), "unexpected error: %v", err) + } + testutil.Equals(t, tc.wantRequests, atomic.LoadInt32(&requests)) + responses := tc.wantRequests + if tc.firstStatus == 0 { + responses-- + } + metrics, err := reg.Gather() + testutil.Ok(t, err) + counts := map[string]float64{} + for _, family := range metrics { + switch family.GetName() { + case "mdox_colly_requests_total": + counts[family.GetName()] = family.GetMetric()[0].GetCounter().GetValue() + case "mdox_colly_per_domain_latency": + metric := family.GetMetric()[0] + testutil.Equals(t, strings.TrimPrefix(srv.URL, "http://"), metric.GetLabel()[0].GetValue()) + counts[family.GetName()] = float64(metric.GetHistogram().GetSampleCount()) + } + } + testutil.Equals(t, map[string]float64{ + "mdox_colly_requests_total": float64(responses), + "mdox_colly_per_domain_latency": float64(responses), + }, counts) + }) + } +} + +func TestValidator_RetryAfter(t *testing.T) { + t.Parallel() + for _, status := range []int{http.StatusTooManyRequests, http.StatusServiceUnavailable} { + for _, format := range []string{"seconds", "HTTP date", "malformed"} { + status, format := status, format + t.Run(fmt.Sprintf("%d/%s", status, format), func(t *testing.T) { + t.Parallel() + var requests int32 + firstResponse := make(chan time.Time, 1) + retriedAt := make(chan time.Time, 1) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + now := time.Now() + if atomic.AddInt32(&requests, 1) > 1 { + retriedAt <- now + w.WriteHeader(http.StatusOK) + return + } + minimum := now.Add(2 * time.Second) + header := "invalid" + switch format { + case "seconds": + header = "3" + minimum = now.Add(3 * time.Second) + case "HTTP date": + minimum = now.Add(4 * time.Second).Truncate(time.Second) + header = minimum.UTC().Format(http.TimeFormat) + } + firstResponse <- minimum + w.Header().Set("Retry-After", header) + w.WriteHeader(status) + })) + defer srv.Close() + + ctx, cancel := context.WithTimeout(context.Background(), 8*time.Second) + defer cancel() + v, err := NewValidator(ctx, log.NewNopLogger(), nil, t.TempDir(), nil, nil) + testutil.Ok(t, err) + testutil.Ok(t, checkRemoteLinks(t, ctx, v, srv.URL)) + testutil.Equals(t, int32(2), atomic.LoadInt32(&requests)) + minimum, actual := <-firstResponse, <-retriedAt + testutil.Assert(t, !actual.Before(minimum), "retried %v before the minimum delay", minimum.Sub(actual)) + }) + } + } +} + +func TestValidator_RetrySchedulingError(t *testing.T) { + t.Parallel() + var requests int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + atomic.AddInt32(&requests, 1) + w.WriteHeader(http.StatusServiceUnavailable) + })) + defer srv.Close() + + v, err := NewValidator(context.Background(), log.NewNopLogger(), nil, t.TempDir(), nil, nil) + testutil.Ok(t, err) + c := v.(*validator).c + // Reject only the retry, after the initial request has passed Colly's checks. + c.OnResponseHeaders(func(_ *colly.Response) { + c.DisallowedURLFilters = []*regexp.Regexp{regexp.MustCompile(".*")} + }) + err = checkRemoteLinks(t, context.Background(), v, srv.URL) + testutil.NotOk(t, err) + testutil.Assert(t, strings.Contains(err.Error(), colly.ErrForbiddenURL.Error()), "unexpected error: %v", err) + testutil.Equals(t, int32(1), atomic.LoadInt32(&requests)) +} + +func TestValidator_LongRetryAfter(t *testing.T) { + t.Parallel() + for _, header := range []string{"61", "3600", time.Now().Add(24 * time.Hour).UTC().Format(http.TimeFormat), "4294967296"} { + header := header + t.Run(header, func(t *testing.T) { + t.Parallel() + var requests int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + atomic.AddInt32(&requests, 1) + w.Header().Set("Retry-After", header) + w.WriteHeader(http.StatusTooManyRequests) + })) + defer srv.Close() + + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + v, err := NewValidator(ctx, log.NewNopLogger(), nil, t.TempDir(), nil, nil) + testutil.Ok(t, err) + err = checkRemoteLinks(t, ctx, v, srv.URL) + testutil.NotOk(t, err) + testutil.Assert(t, strings.Contains(err.Error(), "Retry-After") && strings.Contains(err.Error(), "exceeds maximum"), "unexpected error: %v", err) + testutil.Assert(t, !errors.Is(err, context.DeadlineExceeded), "waited for cancellation instead of reporting the excessive delay") + testutil.Equals(t, int32(1), atomic.LoadInt32(&requests)) + }) + } +} + +func TestValidator_CancelInFlight(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithCancel(context.Background()) + started := make(chan struct{}) + var requests int32 + srv := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + atomic.AddInt32(&requests, 1) + close(started) + <-r.Context().Done() + })) + defer func() { + cancel() + srv.Close() + }() + + v, err := NewValidator(ctx, log.NewNopLogger(), nil, t.TempDir(), nil, nil) + testutil.Ok(t, err) + path := remoteLinksFile(t, srv.URL) + result := make(chan error, 1) + go func() { + _, err := mdformatter.IsFormatted(ctx, log.NewNopLogger(), []string{path}, mdformatter.WithLinkTransformer(v)) + result <- err + }() + + select { + case <-started: + case <-time.After(3 * time.Second): + t.Fatal("request did not start") + } + cancel() + select { + case err := <-result: + testutil.NotOk(t, err) + testutil.Assert(t, errors.Is(err, context.Canceled), "unexpected cancellation error: %v", err) + testutil.Assert(t, !strings.Contains(err.Error(), "backoff"), "in-flight cancellation reported as backoff: %v", err) + case <-time.After(3 * time.Second): + t.Fatal("canceled request did not finish") + } + testutil.Equals(t, int32(1), atomic.LoadInt32(&requests)) +} + +func TestValidator_BackoffDoesNotBlockOtherLinks(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + releaseHealthy := make(chan struct{}) + var requests int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt32(&requests, 1) + if r.URL.Path == "/limited" { + w.Header().Set("Retry-After", "60") + w.WriteHeader(http.StatusTooManyRequests) + return + } + select { + case <-releaseHealthy: + w.WriteHeader(http.StatusOK) + case <-r.Context().Done(): + } + })) + defer func() { + cancel() + srv.Close() + }() + + v, err := NewValidator(ctx, log.NewNopLogger(), nil, t.TempDir(), nil, nil) + testutil.Ok(t, err) + validator := v.(*validator) + limitedResponse := make(chan *colly.Context, 1) + validator.c.OnResponseHeaders(func(r *colly.Response) { + if r.Request.URL.Path == "/limited" { + limitedResponse <- r.Ctx + } + }) + healthyDone := make(chan struct{}) + validator.c.OnScraped(func(r *colly.Response) { + if r.Request.URL.Path == "/healthy" { + close(healthyDone) + } + }) + result := make(chan error, 1) + path := remoteLinksFile(t, srv.URL+"/limited", srv.URL+"/healthy") + go func() { + _, err := mdformatter.IsFormatted(ctx, log.NewNopLogger(), []string{path}, mdformatter.WithLinkTransformer(v)) + result <- err + }() + + ticker := time.NewTicker(5 * time.Millisecond) + defer ticker.Stop() + timer := time.NewTimer(2 * time.Second) + defer timer.Stop() + var requestContext *colly.Context + select { + case requestContext = <-limitedResponse: + case <-timer.C: + t.Fatal("rate-limited response was not received") + } + for requestContext.Get(numberOfRetriesKey) != "1" { + select { + case <-ticker.C: + case <-timer.C: + t.Fatal("rate-limited link did not enter backoff") + } + } + _, err = validator.TransformDestination(mdformatter.SourceContext{Filepath: path, LineNumbers: "duplicate"}, []byte(srv.URL+"/limited")) + testutil.Ok(t, err) + close(releaseHealthy) + select { + case <-healthyDone: + case <-timer.C: + t.Fatal("healthy link was blocked by another link's backoff") + } + cancel() + err = <-result + testutil.NotOk(t, err) + testutil.Assert(t, strings.Contains(err.Error(), context.Canceled.Error()), "unexpected cancellation error: %v", err) + testutil.Assert(t, !strings.Contains(err.Error(), colly.ErrAlreadyVisited.Error()), "duplicate link did not share the final result: %v", err) + testutil.Equals(t, int32(2), atomic.LoadInt32(&requests)) +} + +func TestValidator_ConfiguredTimeout(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + config string + timeout time.Duration + }{ + {name: "default", config: "version: 1\n", timeout: 30 * time.Second}, + {name: "short timeout", config: "version: 1\ntimeout: '100ms'\n", timeout: 100 * time.Millisecond}, + {name: "above previous header timeout", config: "version: 1\ntimeout: '35s'\n", timeout: 35 * time.Second}, + } { + t.Run(tc.name, func(t *testing.T) { + config, err := ParseConfig([]byte(tc.config)) + testutil.Ok(t, err) + testutil.Equals(t, tc.timeout, newHTTPTransport(config).ResponseHeaderTimeout) + }) + } +} diff --git a/pkg/mdformatter/linktransformer/validator.go b/pkg/mdformatter/linktransformer/validator.go index 766f39d..fd5cde9 100644 --- a/pkg/mdformatter/linktransformer/validator.go +++ b/pkg/mdformatter/linktransformer/validator.go @@ -113,7 +113,6 @@ func (v RoundTripValidator) IsValid(k futureKey, r *validator) (bool, error) { } r.l.roundTripVisitedLinks.Inc() - r.c.WithTransport(r.transportFn(k.dest)) if err := r.c.Visit(k.dest); err != nil { r.remoteLinks[k.dest] = fmt.Errorf("remote link %v: %w", k.dest, err) return false, nil