diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..fc2bd56 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,23 @@ +# Contributor instructions + +Keep changes focused and preserve existing behavior unless a change is explicitly requested. Follow CONTRIBUTING.md and document public behavior in README.md. + +## Changelog and pull requests + +Add entries under the current `(Next)` heading in CHANGELOG.md and retain `Your contribution here`. Include the implementation PR number, its URL, and contributor attribution using this format: + +```markdown +* [#123](https://github.com/dblock/ruby-link-checker/pull/123): Describe the change - [@contributor](https://github.com/contributor). +``` + +Predict the PR number before opening a new PR, then verify the assigned number and correct the entry if necessary. Issue references may supplement but do not replace the implementation PR link. + +Keep Markdown paragraphs on one line. Keep each PR branch to one commit, squash follow-up changes before pushing, and use `--force-with-lease` when updating rewritten PR history. Never force-push the default branch. + +## Validation + +Run focused RSpec tests for behavior changes and `bundle exec rubocop` before committing. `bundle exec rake` runs both the full specs and RuboCop. Avoid repeated intermediate test runs; rerun when failures or subsequent changes require it. + +For changelog or README changes, run `bundle exec danger dry_run --base=origin/main`; Danger checks changelog entries and README table-of-contents consistency. Keep the temporary `git < 6` dependency pin until Danger supports git 6; the Gemfile comment links the upstream fix. + +Do not commit Gemfile.lock; this repository ignores it. Include `Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>` in AI-assisted commits and disclose AI involvement when posting GitHub comments on behalf of the user. diff --git a/CHANGELOG.md b/CHANGELOG.md index 54d1562..f787a20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ### 0.5.0 (Next) -* Fixed chained relative redirects, bounded redirect following with `max_redirects`, enabled plain HTTP in the Net::HTTP checker, and corrected retry documentation - [@dblock](https://github.com/dblock), [@Copilot](https://github.com/Copilot). +* [#23](https://github.com/dblock/ruby-link-checker/pull/23): Fixed chained relative redirects, bounded redirect following with `max_redirects`, enabled plain HTTP in the Net::HTTP checker, and corrected retry documentation - [@dblock](https://github.com/dblock), [@Copilot](https://github.com/Copilot). +* [#24](https://github.com/dblock/ruby-link-checker/pull/24): Added transient-only retries, opt-in retry delays/backoff/jitter and `Retry-After` handling, transport error categories, and an Async concurrency limit - [@dblock](https://github.com/dblock), [@Copilot](https://github.com/Copilot). * [#8](https://github.com/dblock/ruby-link-checker/issues/8): Added `protocol` configuration option to `LinkChecker::Async::HTTP` to force HTTP/1 or HTTP/2 - [@dblock](https://github.com/dblock), [@Copilot](https://github.com/Copilot). * Your contribution here. diff --git a/README.md b/README.md index 4d29557..d0df735 100644 --- a/README.md +++ b/README.md @@ -20,6 +20,8 @@ A fast Ruby link checker with support for multiple HTTP libraries. Does not pars - [Options](#options) - [Retries](#retries) - [Redirects](#redirects) + - [Retry After](#retry-after) + - [Retry Policy](#retry-policy) - [Results](#results) - [Methods](#methods) - [Logger](#logger) @@ -180,6 +182,12 @@ LinkChecker::Async::HTTP.configure do |config| end ``` +Async::HTTP reuses connection-pooling clients per scheme/host/port. Its default concurrency is unlimited; set `max_concurrency` to bound active requests without counting delayed retries against that limit. + +```ruby +checker = LinkChecker::Async::HTTP::Checker.new(max_concurrency: 25) +``` + #### [LinkChecker::Net::HTTP](lib/ruby-link-checker/net/http/checker.rb) Slow, sequential checker. @@ -218,7 +226,7 @@ end #### Retries -By default link checkers do not retry. Set `retries` to retry errors and failures. The retry budget is shared across methods: with `retries: 2` and the default methods, repeated failures produce `HEAD`, `HEAD`, `HEAD`, then `GET`. +By default link checkers do not retry. Set `retries` to enable retries for transient connection errors/timeouts and HTTP statuses 408, 425, 429, 500, 502, 503, and 504. Permanent HTTP failures and invalid URLs are not retried. HEAD-to-GET fallback still applies independently of the retry policy. The retry budget is shared across methods: with `retries: 2`, repeated transient failures produce `HEAD`, `HEAD`, `HEAD`, then `GET`. ```ruby checker = LinkChecker::Net::HTTP::Checker.new(retries: 1) @@ -232,6 +240,31 @@ Checkers follow up to 20 redirects per attempt and detect redirect loops. Relati checker = LinkChecker::Typhoeus::Hydra::Checker.new(max_redirects: 10) ``` +#### Retry After + +Set `retry_after` to a minimum delay in seconds (default `0`). Optional `retry_backoff` multiplies that delay on successive retries (default `1`), and `retry_jitter` adds a random fraction between `0` and the configured value (default `0`, maximum `1`). All delays are capped by `max_retry_delay` (default `60` seconds). + +```ruby +checker = LinkChecker::Typhoeus::Hydra::Checker.new( + retries: 2, + retry_after: 1, + retry_backoff: 2, + retry_jitter: 0.1, + respect_retry_after: true, + max_retry_delay: 60 +) +``` + +With `respect_retry_after: true` (default `false`), HTTP `Retry-After` delta-seconds and dates provide a minimum delay, subject to the same cap. Invalid or past dates are ignored. Async delays yield without blocking other requests. Hydra queues delayed retries after the active batch drains, so actual delays can exceed the configured minimum without blocking completion callbacks. Delays apply to retries, not ordinary HEAD-to-GET fallback. + +#### Retry Policy + +Customize `retry_status_codes` to choose retryable HTTP statuses. Set it to `nil` to restore the previous behavior of retrying every failure and error, including malformed URLs and programming errors. + +```ruby +checker = LinkChecker::Net::HTTP::Checker.new(retries: 1, retry_status_codes: [429, 503]) +``` + #### Results By default checkers collect results. @@ -342,6 +375,8 @@ Events are called with results, which contain the following properties. | :request_headers | Request headers. | | :redirect_to | A redirect URL in case of redirects. | | :error | A raised error in case of errors. | +| :error_type | `:timeout`, `:connection`, or `:other` for errors; otherwise `nil`. | +| :response_headers | Response headers, accessible using `[]`. | See [result.rb](lib/ruby-link-checker/result.rb) for more details. diff --git a/lib/ruby-link-checker/async/http/checker.rb b/lib/ruby-link-checker/async/http/checker.rb index 4219d99..3872a9d 100644 --- a/lib/ruby-link-checker/async/http/checker.rb +++ b/lib/ruby-link-checker/async/http/checker.rb @@ -3,7 +3,7 @@ module Async module HTTP class Task < ::LinkChecker::Task def run! - checker._queue { _run! } + checker._queue(delay: delay) { _run! } end private @@ -42,25 +42,36 @@ def initialize(options = {}) end @queue = [] @clients = {} + if max_concurrency && (!max_concurrency.is_a?(Integer) || max_concurrency < 1) + raise ArgumentError, "Invalid concurrency limit: #{max_concurrency}" + end + super end # Runs all queued checks concurrently and blocks until they complete. def run + require 'async/semaphore' + Barrier do |barrier| @barrier = barrier + @semaphore = ::Async::Semaphore.new(max_concurrency) if max_concurrency _flush! end ensure @barrier = nil + @semaphore = nil _close_clients! end - def _queue(&block) + def _queue(delay: 0, &block) if @barrier - @barrier.async(&block) + @barrier.async do + sleep(delay) if delay.positive? + @semaphore ? @semaphore.acquire(&block) : block.call + end else - @queue << block + @queue << [delay, block] end end @@ -94,7 +105,7 @@ def _endpoint_options def _flush! queue = @queue @queue = [] - queue.each { |block| @barrier.async(&block) } + queue.each { |delay, block| _queue(delay: delay, &block) } end def _close_clients! diff --git a/lib/ruby-link-checker/async/http/config.rb b/lib/ruby-link-checker/async/http/config.rb index f21d8e1..32b2930 100644 --- a/lib/ruby-link-checker/async/http/config.rb +++ b/lib/ruby-link-checker/async/http/config.rb @@ -10,6 +10,7 @@ module Config read_timeout open_timeout protocol + max_concurrency ].freeze attr_accessor(*Config::ATTRIBUTES) @@ -18,6 +19,7 @@ def reset self.read_timeout = nil self.open_timeout = nil self.protocol = nil + self.max_concurrency = nil end end diff --git a/lib/ruby-link-checker/async/http/result.rb b/lib/ruby-link-checker/async/http/result.rb index 4a686a4..1d7a3c4 100644 --- a/lib/ruby-link-checker/async/http/result.rb +++ b/lib/ruby-link-checker/async/http/result.rb @@ -58,6 +58,10 @@ def request_headers Headers.new(request.headers) end + def response_headers + response&.headers || {} + end + def redirect_to return nil unless response diff --git a/lib/ruby-link-checker/checker.rb b/lib/ruby-link-checker/checker.rb index 44e4327..129e0bd 100644 --- a/lib/ruby-link-checker/checker.rb +++ b/lib/ruby-link-checker/checker.rb @@ -6,11 +6,10 @@ class Checker include LinkChecker::Callbacks attr_reader :results - attr_accessor(*(Config::ATTRIBUTES - %i[max_redirects])) def initialize(options = {}) LinkChecker::Config::ATTRIBUTES.each do |key| - send("#{key}=", options[key] || LinkChecker.config.send(key)) + send("#{key}=", options.fetch(key) { LinkChecker.config.send(key) }) end raise ArgumentError, "Missing methods." if methods&.none? @logger ||= options[:logger] || LinkChecker::Config.logger || LinkChecker::Logger.default diff --git a/lib/ruby-link-checker/config.rb b/lib/ruby-link-checker/config.rb index 05d36f3..20eba0c 100644 --- a/lib/ruby-link-checker/config.rb +++ b/lib/ruby-link-checker/config.rb @@ -11,6 +11,12 @@ module Config logger retries max_redirects + retry_status_codes + retry_after + retry_backoff + retry_jitter + max_retry_delay + respect_retry_after ].freeze attr_accessor(*Config::ATTRIBUTES) @@ -22,6 +28,12 @@ def reset self.logger = nil self.retries = 0 self.max_redirects = 20 + self.retry_status_codes = [408, 425, 429, 500, 502, 503, 504] + self.retry_after = 0 + self.retry_backoff = 1 + self.retry_jitter = 0 + self.max_retry_delay = 60 + self.respect_retry_after = false end def retries=(value) @@ -35,6 +47,44 @@ def max_redirects=(value) @max_redirects = value end + + def retry_status_codes=(value) + unless value.nil? || (value.is_a?(Array) && value.all? { |code| code.is_a?(Integer) && code.between?(100, 599) }) + raise ArgumentError, "Invalid retry status codes: #{value}" + end + + @retry_status_codes = value&.dup + end + + def retry_after=(value) + validate_number!(:retry_after, value, 0) + @retry_after = value + end + + def retry_backoff=(value) + validate_number!(:retry_backoff, value, 1) + @retry_backoff = value + end + + def retry_jitter=(value) + validate_number!(:retry_jitter, value, 0) + raise ArgumentError, "Invalid retry_jitter: #{value}" if value > 1 + + @retry_jitter = value + end + + def max_retry_delay=(value) + validate_number!(:max_retry_delay, value, 0) + @max_retry_delay = value + end + + private + + def validate_number!(name, value, minimum) + return if value.is_a?(Numeric) && value.real? && value.finite? && value >= minimum + + raise ArgumentError, "Invalid #{name}: #{value}" + end end class << self diff --git a/lib/ruby-link-checker/errors.rb b/lib/ruby-link-checker/errors.rb index 1a5ea2c..d5592a4 100644 --- a/lib/ruby-link-checker/errors.rb +++ b/lib/ruby-link-checker/errors.rb @@ -1,3 +1,4 @@ require_relative 'errors/base_error' require_relative 'errors/redirect_loop_error' require_relative 'errors/redirect_limit_error' +require_relative 'errors/connection_error' diff --git a/lib/ruby-link-checker/errors/connection_error.rb b/lib/ruby-link-checker/errors/connection_error.rb new file mode 100644 index 0000000..9b8c860 --- /dev/null +++ b/lib/ruby-link-checker/errors/connection_error.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module LinkChecker + module Errors + class ConnectionError < BaseError + TRANSIENT_CODES = %i[ + couldnt_resolve_proxy couldnt_resolve_host couldnt_connect + partial_file got_nothing send_error recv_error http2 http2_stream + ].freeze + + attr_reader :return_code + + def initialize(return_code, message) + @return_code = return_code + super(message) + end + + def error_type + TRANSIENT_CODES.include?(return_code) ? :connection : :other + end + end + end +end diff --git a/lib/ruby-link-checker/net/http/checker.rb b/lib/ruby-link-checker/net/http/checker.rb index 81aaab5..6691525 100644 --- a/lib/ruby-link-checker/net/http/checker.rb +++ b/lib/ruby-link-checker/net/http/checker.rb @@ -3,6 +3,7 @@ module Net module HTTP class Task < ::LinkChecker::Task def run! + sleep(delay) if delay.positive? ::Net::HTTP.start(uri.host, uri.port, use_ssl: uri.scheme == 'https') do |http| http.read_timeout = checker.read_timeout if checker.read_timeout http.open_timeout = checker.open_timeout if checker.open_timeout diff --git a/lib/ruby-link-checker/net/http/result.rb b/lib/ruby-link-checker/net/http/result.rb index 6e85342..cc9351c 100644 --- a/lib/ruby-link-checker/net/http/result.rb +++ b/lib/ruby-link-checker/net/http/result.rb @@ -30,6 +30,10 @@ def request_headers request end + def response_headers + response || {} + end + def redirect_to return nil unless response diff --git a/lib/ruby-link-checker/result.rb b/lib/ruby-link-checker/result.rb index d6d808b..8bfa082 100644 --- a/lib/ruby-link-checker/result.rb +++ b/lib/ruby-link-checker/result.rb @@ -1,3 +1,6 @@ +require 'timeout' +require 'socket' + module LinkChecker class Result attr_accessor :uri, :result_uri, :method, :options, :checker @@ -33,6 +36,14 @@ def request_headers {} end + def response_headers + {} + end + + def error_type + nil + end + def code nil end @@ -70,5 +81,14 @@ def error? def code error.class.name end + + def error_type + return :timeout if error.is_a?(Timeout::Error) + return :timeout if defined?(::Async::TimeoutError) && error.is_a?(::Async::TimeoutError) + return error.error_type if error.is_a?(LinkChecker::Errors::ConnectionError) + return :connection if [IOError, SystemCallError, SocketError].any? { |klass| error.is_a?(klass) } + + :other + end end end diff --git a/lib/ruby-link-checker/task.rb b/lib/ruby-link-checker/task.rb index 69dd20e..64d48dd 100644 --- a/lib/ruby-link-checker/task.rb +++ b/lib/ruby-link-checker/task.rb @@ -3,6 +3,7 @@ class Task include LinkChecker::Callbacks attr_reader :uri, :original_uri, :method, :logger, :options, :checker + attr_accessor :delay def initialize(checker, uri, method, original_uri, options = {}) @checker = checker @@ -11,6 +12,7 @@ def initialize(checker, uri, method, original_uri, options = {}) @original_uri = original_uri || @uri @method = method @options = options + @delay = 0 end def run! diff --git a/lib/ruby-link-checker/tasks.rb b/lib/ruby-link-checker/tasks.rb index 3f95987..bbdadd9 100644 --- a/lib/ruby-link-checker/tasks.rb +++ b/lib/ruby-link-checker/tasks.rb @@ -1,3 +1,5 @@ +require 'time' + module LinkChecker class Tasks include LinkChecker::Callbacks @@ -7,6 +9,7 @@ class Tasks def initialize(checker, task_klass, uri, methods, options = {}) @uri = uri @retries_left = checker.retries + @retry_count = 0 @methods_left = methods.dup @methods = methods.dup @task_klass = task_klass @@ -24,8 +27,9 @@ def new_task(uri, method, original_uri, options) def execute! if retry? @retries_left -= 1 + @retry_count += 1 retry! @result - _queue_task(uri, method, original_uri || uri, options) + _queue_task(uri, method, original_uri || uri, options, delay: _retry_delay) elsif methods_left.any? @method = methods_left.shift @uri = URI(@uri) unless @uri.is_a?(URI) @@ -58,11 +62,37 @@ def retries_left end def retry? - !first_time? && retries_left > 0 + !first_time? && retries_left > 0 && retryable? + end + + def retryable? + return false unless result + return result.error? || result.failure? if checker.retry_status_codes.nil? + return %i[timeout connection].include?(result.error_type) if result.error? + + result.failure? && checker.retry_status_codes.include?(result.code) + end + + def _retry_delay + delay = checker.retry_after.zero? ? 0 : checker.retry_after * (checker.retry_backoff**(@retry_count - 1)) + delay = [delay, _server_retry_delay].max if checker.respect_retry_after + delay *= 1 + (rand * checker.retry_jitter) if checker.retry_jitter.positive? + [delay, checker.max_retry_delay].min + end + + def _server_retry_delay + headers = result.response_headers + value = Array(headers['Retry-After'] || headers['retry-after']).first.to_s.strip + return value.to_i if /\A\d+\z/.match?(value) + + [Time.httpdate(value) - Time.now, 0].max + rescue ArgumentError + 0 end - def _queue_task(uri, method, original_uri, options = {}) + def _queue_task(uri, method, original_uri, options = {}, delay: 0) task = new_task(uri, method, original_uri, options) + task.delay = delay task.on :result do |result| _handle_result result end diff --git a/lib/ruby-link-checker/typhoeus/hydra/checker.rb b/lib/ruby-link-checker/typhoeus/hydra/checker.rb index d41afd5..242702e 100644 --- a/lib/ruby-link-checker/typhoeus/hydra/checker.rb +++ b/lib/ruby-link-checker/typhoeus/hydra/checker.rb @@ -5,7 +5,7 @@ class Task < ::LinkChecker::Task def run! request = ::Typhoeus::Request.new( uri, { - method: method, + method: method.downcase.to_sym, followlocation: false, timeout: checker.timeout, connecttimeout: checker.connecttimeout, @@ -17,11 +17,14 @@ def run! request.on_complete do |response| if response.timed_out? result! ResultError.new(uri, method, original_uri, Timeout::Error.new, options) + elsif response.code.zero? + error = LinkChecker::Errors::ConnectionError.new(response.return_code, response.return_message) + result! ResultError.new(uri, method, original_uri, error, options) else result! Result.new(uri, method, original_uri, request, response, options) end end - checker._queue(request) + checker._queue(request, delay: delay) end end @@ -35,15 +38,34 @@ def initialize(options = {}) send("#{key}=", options[key] || LinkChecker::Typhoeus::Hydra::Config.send(key)) end @hydra = ::Typhoeus::Hydra.new(options[:hydra] || { max_concurrency: 10 }) + @delayed = [] super options end def run - @hydra.run + loop do + @hydra.run + break if @delayed.empty? + + deadline = @delayed.map(&:first).min + sleep([deadline - monotonic_time, 0].max) + ready, @delayed = @delayed.partition { |time, _request| time <= monotonic_time } + ready.each { |entry| @hydra.queue(entry.last) } + end + end + + def _queue(request, delay: 0) + if delay.positive? + @delayed << [monotonic_time + delay, request] + else + @hydra.queue(request) + end end - def _queue(request) - @hydra.queue(request) + private + + def monotonic_time + Process.clock_gettime(Process::CLOCK_MONOTONIC) end end end diff --git a/lib/ruby-link-checker/typhoeus/hydra/result.rb b/lib/ruby-link-checker/typhoeus/hydra/result.rb index e107c77..1c4becb 100644 --- a/lib/ruby-link-checker/typhoeus/hydra/result.rb +++ b/lib/ruby-link-checker/typhoeus/hydra/result.rb @@ -30,6 +30,10 @@ def request_headers request.options[:headers] end + def response_headers + response&.headers || {} + end + def redirect_to return nil unless response diff --git a/spec/ruby-link-checker/async/http/checker_spec.rb b/spec/ruby-link-checker/async/http/checker_spec.rb index 2ce8da7..098b78f 100644 --- a/spec/ruby-link-checker/async/http/checker_spec.rb +++ b/spec/ruby-link-checker/async/http/checker_spec.rb @@ -33,6 +33,43 @@ def check(url, options = {}) LinkChecker::Async::HTTP::Config.reset end + describe 'concurrency' do + let(:checker) { described_class.new(max_concurrency: 2) } + let(:counts) { [] } + let(:completed) { [] } + + before do + active = 0 + 5.times do + checker._queue do + counts << (active += 1) + sleep(0.005) + active -= 1 + completed << active + end + end + end + + it 'bounds active work and waits for queued tasks' do + checker.run + expect(counts.max).to eq(2) + expect(counts.size).to eq(5) + expect(completed.size).to eq(5) + expect(completed.last).to eq(0) + end + + it 'rejects invalid limits' do + expect { described_class.new(max_concurrency: 0) }.to raise_error ArgumentError + end + + it 'reuses clients for the same endpoint' do + expect(checker._client_for(URI('https://www.example.org/one'))).to equal( + checker._client_for(URI('https://www.example.org/two')) + ) + checker.run + end + end + describe AsyncTestLinkChecker::LinkChecker do it_behaves_like 'a link checker' diff --git a/spec/ruby-link-checker/checker_spec.rb b/spec/ruby-link-checker/checker_spec.rb index 4a88f9e..c759cc3 100644 --- a/spec/ruby-link-checker/checker_spec.rb +++ b/spec/ruby-link-checker/checker_spec.rb @@ -10,5 +10,21 @@ it 'rejects an invalid redirect limit' do expect { described_class.new(max_redirects: -1) }.to raise_error ArgumentError, 'Invalid redirect limit: -1' end + + it 'validates retry delays and backoff' do + expect { described_class.new(retry_after: -1) }.to raise_error ArgumentError + expect { described_class.new(retry_backoff: 0.5) }.to raise_error ArgumentError + expect { described_class.new(retry_jitter: 2) }.to raise_error ArgumentError + expect { described_class.new(max_retry_delay: Float::INFINITY) }.to raise_error ArgumentError + end + + it 'rejects invalid retry status codes' do + expect { described_class.new(retry_status_codes: ['503']) }.to raise_error ArgumentError + end + + it 'allows per-instance disabling of global Retry-After handling' do + LinkChecker.config.respect_retry_after = true + expect(described_class.new(respect_retry_after: false).respect_retry_after).to be false + end end end \ No newline at end of file diff --git a/spec/ruby-link-checker/result_error_spec.rb b/spec/ruby-link-checker/result_error_spec.rb index 44d4897..1b15645 100644 --- a/spec/ruby-link-checker/result_error_spec.rb +++ b/spec/ruby-link-checker/result_error_spec.rb @@ -14,4 +14,21 @@ it 'returns the error class name as code' do expect(result.code).to eq 'StandardError' end + + it 'classifies native timeouts without replacing the exception' do + error = Net::ReadTimeout.new('timeout') + result.error = error + expect(result.error_type).to eq(:timeout) + expect(result.error).to equal(error) + end + + it 'classifies native connection failures' do + result.error = Errno::ECONNREFUSED.new + expect(result.error_type).to eq(:connection) + end + + it 'does not classify programming errors as transient' do + result.error = ArgumentError.new + expect(result.error_type).to eq(:other) + end end diff --git a/spec/ruby-link-checker/tasks_spec.rb b/spec/ruby-link-checker/tasks_spec.rb index fbec879..e5ec37f 100644 --- a/spec/ruby-link-checker/tasks_spec.rb +++ b/spec/ruby-link-checker/tasks_spec.rb @@ -3,6 +3,50 @@ require 'spec_helper' describe LinkChecker::Tasks do + describe 'retry delays' do + let(:checker) { LinkChecker::Net::HTTP::Checker.new } + let(:tasks) { described_class.new(checker, LinkChecker::Net::HTTP::Task, 'https://www.example.org', ['GET']) } + let(:headers) { { 'Retry-After' => '60' } } + let(:result) { instance_double(LinkChecker::Net::HTTP::Result, response_headers: headers) } + + before do + allow(tasks).to receive(:result).and_return(result) + end + + it 'parses delta-seconds' do + expect(tasks.send(:_server_retry_delay)).to eq(60) + end + + context 'with a past HTTP date' do + let(:headers) { { 'Retry-After' => (Time.now - 60).httpdate } } + + it 'does not delay a retry' do + expect(tasks.send(:_server_retry_delay)).to eq(0) + end + end + + context 'with an invalid negative delay' do + let(:headers) { { 'Retry-After' => '-5' } } + + it 'does not delay a retry' do + expect(tasks.send(:_server_retry_delay)).to eq(0) + end + end + + it 'caps configured delays' do + checker.retry_after = 120 + checker.max_retry_delay = 10 + expect(tasks.send(:_retry_delay)).to eq(10) + end + + it 'adds jitter without reducing the minimum delay' do + checker.retry_after = 2 + checker.retry_jitter = 0.5 + allow(tasks).to receive(:rand).and_return(0.5) + expect(tasks.send(:_retry_delay)).to eq(2.5) + end + end + describe '#_handle_result' do # Reproduces issue #12: SystemStackError when logging causes repeated errors # https://github.com/dblock/ruby-link-checker/issues/12 diff --git a/spec/ruby-link-checker/typhoeus/hydra/checker_spec.rb b/spec/ruby-link-checker/typhoeus/hydra/checker_spec.rb index 8e823d1..3f33233 100644 --- a/spec/ruby-link-checker/typhoeus/hydra/checker_spec.rb +++ b/spec/ruby-link-checker/typhoeus/hydra/checker_spec.rb @@ -90,4 +90,51 @@ def check(url, options = {}) checker.run end end + + describe 'delayed retries' do + let(:checker) { described_class.new(methods: ['GET'], retries: 1, retry_after: 0.005) } + + around do |example| + VCR.turned_off { example.run } + end + + it 'processes unrelated requests before waiting for a retry' do + completed = [] + stub_request(:get, 'https://www.example.org/retry').to_return({ status: 503 }, { status: 200 }) + stub_request(:get, 'https://www.example.org/other').to_return(status: 200) + checker.on(:success) { |result| completed << result.uri.path } + checker.check('https://www.example.org/retry') + checker.check('https://www.example.org/other') + checker.run + expect(completed).to eq(['/other', '/retry']) + end + end + + describe 'transport failures' do + let(:checker) { described_class.new(methods: ['GET']) } + + around do |example| + VCR.turned_off do + WebMock.disable! + Typhoeus::Config.block_connection = true + example.run + ensure + Typhoeus::Config.block_connection = false + WebMock.enable! + end + end + + after do + Typhoeus::Expectation.clear + end + + it 'reports connection errors separately from HTTP failures' do + response = Typhoeus::Response.new(code: 0, return_code: :couldnt_connect) + Typhoeus.stub(nil).and_return(response) + checker.check('https://www.example.org/') + checker.run + expect(checker.results[:failure]).to be_empty + expect(checker.results[:error].first.error_type).to eq(:connection) + end + end end diff --git a/spec/support/link_checker.rb b/spec/support/link_checker.rb index a1a8782..6cded56 100644 --- a/spec/support/link_checker.rb +++ b/spec/support/link_checker.rb @@ -50,6 +50,93 @@ end end + context 'with transient retries' do + let(:checker_options) { {} } + let(:checker) { described_class.new({ methods: ['GET'], retries: 2 }.merge(checker_options)) } + let(:url) { 'https://www.example.org/retry' } + + around do |example| + VCR.turned_off { example.run } + end + + it 'retries service errors and preserves metadata' do + stub_request(:get, url).to_return({ status: 503 }, { status: 200 }) + checker.check(url, location: 'page.html') + checker.run if checker.respond_to?(:run) + expect(checker.results[:success].first.options).to eq(location: 'page.html') + expect(a_request(:get, url)).to have_been_made.twice + end + + it 'does not retry permanent HTTP failures' do + stub_request(:get, url).to_return(status: 404) + checker.check(url) + checker.run if checker.respond_to?(:run) + expect(checker.results[:failure].first.code).to eq(404) + expect(a_request(:get, url)).to have_been_made.once + end + + it 'retries timeouts and reports a timeout category' do + stub_request(:get, url).to_timeout + checker.check(url) + checker.run if checker.respond_to?(:run) + expect(checker.results[:error].first.error_type).to eq(:timeout) + expect(a_request(:get, url)).to have_been_made.times(3) + end + + it 'does not retry malformed URLs' do + checker.check('\/invalid-url') + checker.run if checker.respond_to?(:run) + expect(checker.results[:error].first.error_type).to eq(:other) + expect(checker.results[:error].size).to eq(1) + end + + context 'with a configured delay and backoff' do + let(:checker_options) { { retry_after: 0.005, retry_backoff: 2 } } + + it 'waits before each retry without changing the retry budget' do + stub_request(:get, url).to_return({ status: 503 }, { status: 503 }, { status: 200 }) + started = Process.clock_gettime(Process::CLOCK_MONOTONIC) + checker.check(url) + checker.run if checker.respond_to?(:run) + elapsed = Process.clock_gettime(Process::CLOCK_MONOTONIC) - started + expect(elapsed).to be >= 0.015 + expect(a_request(:get, url)).to have_been_made.times(3) + expect(checker.results[:success].size).to eq(1) + end + end + + context 'with Retry-After enabled' do + let(:checker_options) { { respect_retry_after: true, max_retry_delay: 0.005 } } + + it 'honors a bounded server delay' do + stub_request(:get, url).to_return({ status: 429, headers: { 'Retry-After' => '5' } }, { status: 200 }) + started = Process.clock_gettime(Process::CLOCK_MONOTONIC) + checker.check(url) + checker.run if checker.respond_to?(:run) + elapsed = Process.clock_gettime(Process::CLOCK_MONOTONIC) - started + expect(elapsed).to be >= 0.005 + expect(elapsed).to be < 1 + expect(checker.results[:success].size).to eq(1) + end + + it 'accepts HTTP dates' do + stub_request(:get, url).to_return( + { status: 503, headers: { 'Retry-After' => (Time.now + 60).httpdate } }, { status: 200 } + ) + checker.check(url) + checker.run if checker.respond_to?(:run) + expect(checker.results[:success].size).to eq(1) + end + + it 'ignores invalid server delays' do + stub_request(:get, url).to_return({ status: 503, headers: { 'Retry-After' => 'invalid' } }, { status: 200 }) + checker.check(url) + checker.run if checker.respond_to?(:run) + expect(checker.results[:success].size).to eq(1) + end + end + end + context 'default' do subject do described_class.new @@ -227,7 +314,7 @@ context 'with 1 retry' do subject do - described_class.new(methods: ['GET'], retries: 1) + described_class.new(methods: ['GET'], retries: 1, retry_status_codes: nil) end it 'fails' do @@ -243,7 +330,7 @@ context 'with 2 retries' do subject do - described_class.new(methods: ['GET'], retries: 2) + described_class.new(methods: ['GET'], retries: 2, retry_status_codes: nil) end it 'fails' do