diff --git a/app/web/errors/error_classifier.rb b/app/web/errors/error_classifier.rb index 9776a1f1..b8b42e40 100644 --- a/app/web/errors/error_classifier.rb +++ b/app/web/errors/error_classifier.rb @@ -19,10 +19,23 @@ module ErrorClassifier # rubocop:disable Metrics/ModuleLength -- Decision catalo # Immutable HTTP decision for a classified error. Decision = Data.define( :status, :code, :message, :kind, :cacheable, :retryable, :next_action, :retry_action - ) + ) do + # Maps <500 classified decisions to their empty RenderResult reason. + # + # @return [String] + def empty_reason + case code + when BLOCKED_SURFACE_CODE then 'blocked_surface' + when 'FORBIDDEN' then 'forbidden' + else 'content_extraction_empty' + end + end + end ## # Gem strategy attempts plus expanded transport telemetry for feed Observability / headers. + TRANSPORT_KEYS = %i[request_id strategy_used render_ms error_category timeout_phase].freeze + Diagnostics = Data.define( :strategy_attempts, :request_id, :strategy_used, :render_ms, :error_category, :timeout_phase, :surface_category @@ -39,14 +52,15 @@ def empty # # @param error [Exception, nil] # @return [Html2rss::Web::ErrorClassifier::Diagnostics] - def from_error(error) # rubocop:disable Metrics/AbcSize -- chain dig + surface_category + def from_error(error) # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity -- chain dig carried = decided_diagnostics(error) return carried if carried chain = ErrorClassifier.error_chain(error) - with_attempts = chain.find { it.is_a?(::Html2rss::NoFeedItemsExtracted) } + with_attempts = chain.find { it.is_a?(::Html2rss::Error) && Array(it.attempts).any? } + meta_error = chain.find { it.is_a?(::Html2rss::Error) && it.transport_meta&.any? } list = with_attempts ? Array(with_attempts.attempts) : [] - fields = transport_fields(list) + fields = transport_fields(list, fallback_meta: meta_error&.transport_meta) fields[:timeout_phase] ||= timeout_phase_from_chain(chain) fields[:surface_category] = surface_category_from_chain(chain) @@ -78,16 +92,11 @@ def decided_diagnostics(error) end # @param attempts [Array] + # @param fallback_meta [Hash, nil] # @return [Hash{Symbol=>Object}] - def transport_fields(attempts) - meta = attempts.filter_map { it[:transport_meta] || it['transport_meta'] }.last - { - request_id: dig_meta(meta, :request_id), - strategy_used: dig_meta(meta, :strategy_used), - render_ms: dig_meta(meta, :render_ms), - error_category: dig_meta(meta, :error_category), - timeout_phase: dig_meta(meta, :timeout_phase) - } + def transport_fields(attempts, fallback_meta: nil) + meta = attempts.filter_map { it[:transport_meta] || it['transport_meta'] }.last || fallback_meta + TRANSPORT_KEYS.to_h { [it, dig_meta(meta, it)] } end # @param chain [Array] @@ -116,12 +125,7 @@ def dig_meta(meta, key) # # @return [Hash{Symbol=>Object}] def to_h - details = strategy_attempts.empty? ? {} : { strategy_attempts: strategy_attempts } - %i[request_id strategy_used render_ms error_category timeout_phase surface_category].each do |key| - val = public_send(key) - details[key] = val if val - end - details + super.reject { |_, val| val.nil? || val == [] } end end EMPTY_DIAGNOSTICS = Diagnostics.new( @@ -159,7 +163,12 @@ def initialize(decision, diagnostics: Diagnostics.empty) # Non-timeout transport failures → internal network decision when unclassified. NETWORK_ERRORS = [ SocketError, EOFError, Errno::ECONNREFUSED, Errno::EHOSTUNREACH, - HTTPX::ConnectionError, HTTPX::TLSError + HTTPX::ConnectionError, HTTPX::TLSError, + *(if ::Html2rss::RequestService.const_defined?(:ConnectionFailed, false) + [::Html2rss::RequestService::ConnectionFailed] + else + [] + end) ].freeze AUTH_META = { kind: 'auth', retryable: false, next_action: 'enter_token', retry_action: 'none' }.freeze @@ -175,11 +184,8 @@ def initialize(decision, diagnostics: Diagnostics.empty) status: 422, code: EXTRACTION_EMPTY_CODE, message: EXTRACTION_EMPTY_MESSAGE, - kind: 'input', cacheable: true, - retryable: false, - next_action: 'correct_input', - retry_action: 'none' + **INPUT_META ).freeze # Shell / high-entropy empties stay a plain 422 but must not be stored. @@ -191,15 +197,11 @@ def initialize(decision, diagnostics: Diagnostics.empty) BLOCKED_SURFACE_CODE = 'BLOCKED_SURFACE' BLOCKED_SURFACE_MESSAGE = 'This site blocked automated access. Try another URL or site.' + # Anti-bot challenge surfaces deterministically block automated fetches for this IP; + # cacheable: true avoids hammering challenge endpoints during the feed TTL. BLOCKED_SURFACE = Decision.new( - status: 422, - code: BLOCKED_SURFACE_CODE, - message: BLOCKED_SURFACE_MESSAGE, - kind: 'input', - cacheable: true, - retryable: false, - next_action: 'correct_input', - retry_action: 'none' + status: 422, code: BLOCKED_SURFACE_CODE, message: BLOCKED_SURFACE_MESSAGE, + cacheable: true, **INPUT_META ).freeze SCRAPER_UNAVAILABLE_CODE = 'SCRAPER_UNAVAILABLE' @@ -211,9 +213,7 @@ def initialize(decision, diagnostics: Diagnostics.empty) message: SCRAPER_UNAVAILABLE_MESSAGE, kind: 'server', cacheable: false, - retryable: true, - next_action: 'retry', - retry_action: 'primary' + **RETRY_META ).freeze SERVICE_UNAVAILABLE = Decision.new( @@ -222,9 +222,7 @@ def initialize(decision, diagnostics: Diagnostics.empty) message: 'The server is too busy or the request timed out.', kind: 'server', cacheable: false, - retryable: true, - next_action: 'retry', - retry_action: 'primary' + **RETRY_META ).freeze GATEWAY_TIMEOUT = Decision.new( @@ -233,9 +231,7 @@ def initialize(decision, diagnostics: Diagnostics.empty) message: 'The target website took too long to respond.', kind: 'network', cacheable: false, - retryable: true, - next_action: 'retry', - retry_action: 'primary' + **RETRY_META ).freeze INTERNAL_SERVER_ERROR = Decision.new( @@ -244,20 +240,15 @@ def initialize(decision, diagnostics: Diagnostics.empty) message: 'Internal Server Error', kind: 'server', cacheable: false, - retryable: true, - next_action: 'retry', - retry_action: 'primary' + **RETRY_META ).freeze PRIVATE_NETWORK_DENIED = Decision.new( status: 403, code: 'FORBIDDEN', message: 'Requests to private or loopback networks are forbidden.', - kind: 'input', cacheable: true, - retryable: false, - next_action: 'correct_input', - retry_action: 'none' + **INPUT_META ).freeze INTERNAL_NETWORK_ERROR = Decision.new( @@ -266,9 +257,7 @@ def initialize(decision, diagnostics: Diagnostics.empty) message: 'Internal Server Error', kind: 'network', cacheable: false, - retryable: true, - next_action: 'retry', - retry_action: 'primary' + **RETRY_META ).freeze class << self diff --git a/app/web/feeds/contracts.rb b/app/web/feeds/contracts.rb index 2caf22d1..5573b388 100644 --- a/app/web/feeds/contracts.rb +++ b/app/web/feeds/contracts.rb @@ -103,6 +103,11 @@ def http_status def client_message decision.message end + + # @return [Boolean] + def cacheable? + decision.nil? || decision.cacheable + end end end end diff --git a/app/web/feeds/renderer.rb b/app/web/feeds/renderer.rb index 35034821..97ea599b 100644 --- a/app/web/feeds/renderer.rb +++ b/app/web/feeds/renderer.rb @@ -143,7 +143,7 @@ def plain_response?(result) # @param result [Html2rss::Web::Feeds::Contracts::RenderResult] # @return [void] def apply_cache_headers(response, result) - return expires_now(response) unless result.decision.nil? || result.decision.cacheable + return expires_now(response) unless result.cacheable? expires(response, result.ttl_seconds, cache_control: 'public') end diff --git a/app/web/feeds/service.rb b/app/web/feeds/service.rb index 217a9398..6294434c 100644 --- a/app/web/feeds/service.rb +++ b/app/web/feeds/service.rb @@ -7,10 +7,6 @@ module Feeds # Shared synchronous feed service around the html2rss gem. module Service class << self - EMPTY_REASON_BY_CODE = { - ErrorClassifier::BLOCKED_SURFACE_CODE => 'blocked_surface', - 'FORBIDDEN' => 'forbidden' - }.freeze # @param resolved_source [Html2rss::Web::Feeds::Contracts::ResolvedSource] # @return [Html2rss::Web::Feeds::Contracts::RenderResult] def call(resolved_source) @@ -19,7 +15,7 @@ def call(resolved_source) Cache.fetch( cache_key, ttl_seconds: resolved_source.ttl_seconds, - cacheable: ->(result) { result.decision.nil? || result.decision.cacheable } + cacheable: lambda(&:cacheable?) ) do build_result(resolved_source, cache_key) end @@ -116,7 +112,7 @@ def classified_empty_result(error, decision, diagnostics, resolved_source, cache cache_key:, error_message: error.message, error_class: error.class.name, - empty_reason: EMPTY_REASON_BY_CODE.fetch(decision.code, 'content_extraction_empty') + empty_reason: decision.empty_reason ) end diff --git a/docs/README.md b/docs/README.md index 7ac688af..e238b392 100644 --- a/docs/README.md +++ b/docs/README.md @@ -265,7 +265,7 @@ Start from the newest `feed.create`, `feed.render`, and `request.error` lines. F 3. **Decision tree** - `SCRAPER_UNAVAILABLE` → scraper down or unreachable - `SERVICE_UNAVAILABLE` with queue/boot timeout wording in `details.error_message` → our capacity or Chromium boot, not the target site - - `BLOCKED_SURFACE` → site blocked automation (product signal) + - `BLOCKED_SURFACE` → site blocked automation (product signal; cached for feed TTL with `details.cacheable=true` to avoid hammering challenge pages) - `EXTRACTION_EMPTY` → selectors/config (product signal) - `GATEWAY_TIMEOUT` → slow or hostile target, or timeout ladder mismatch (see **Compose timeout ladder**) diff --git a/spec/html2rss/web/error_classifier_spec.rb b/spec/html2rss/web/error_classifier_spec.rb index bc15cbc7..c94dcdbb 100644 --- a/spec/html2rss/web/error_classifier_spec.rb +++ b/spec/html2rss/web/error_classifier_spec.rb @@ -81,6 +81,21 @@ expect(described_class.classify(error)).to eq(described_class::BLOCKED_SURFACE) end + it 'exposes blocked-surface cache policy and empty reason', :aggregate_failures do + expect(described_class::BLOCKED_SURFACE.cacheable).to be(true) + expect(described_class::BLOCKED_SURFACE.empty_reason).to eq('blocked_surface') + end + + it 'classifies ConnectionFailed as internal network error', :aggregate_failures, + if: Html2rss::RequestService.const_defined?(:ConnectionFailed, false) do + expect(described_class.classify(Html2rss::RequestService::ConnectionFailed.new('refused'))).to eq( + described_class::INTERNAL_NETWORK_ERROR + ) + expect(described_class.classify(Html2rss::RequestService::ConnectionFailed.new('refused'))).to have_attributes( + status: 500, kind: 'network', cacheable: false + ) + end + it 'uses one human sentence for classified user decisions', :aggregate_failures do expect(described_class::BLOCKED_SURFACE.message).to eq( 'This site blocked automated access. Try another URL or site.' @@ -304,6 +319,43 @@ expect(described_class::Diagnostics.from_error(decided)).to eq(carried) end + + it 'extracts attempts from RequestTimedOut on the chain', :aggregate_failures do + attempts = [{ strategy: :default, error_class: 'Html2rss::RequestService::RequestTimedOut', elapsed_ms: 12 }] + error = Html2rss::RequestService::RequestTimedOut.new('timed out', timeout_phase: 'work') + error.attempts = attempts + + diagnostics = described_class::Diagnostics.from_error(error) + expect(diagnostics.strategy_attempts).to eq(attempts) + expect(diagnostics.timeout_phase).to eq('work') + end + + it 'extracts attempts from BlockedSurfaceDetected on the chain', :aggregate_failures do + attempts = [{ strategy: :botasaurus, error_class: 'Html2rss::RequestService::BlockedSurfaceDetected' }] + error = Html2rss::RequestService::BlockedSurfaceDetected.new('challenge') + error.attempts = attempts + + expect(described_class::Diagnostics.from_error(error).strategy_attempts).to eq(attempts) + end + + it 'extracts attempts from ConnectionFailed on the chain', :aggregate_failures, + if: Html2rss::RequestService.const_defined?(:ConnectionFailed, false) do + attempts = [{ strategy: :default, error_class: 'Html2rss::RequestService::ConnectionFailed' }] + error = Html2rss::RequestService::ConnectionFailed.new('refused') + error.attempts = attempts + + expect(described_class::Diagnostics.from_error(error).strategy_attempts).to eq(attempts) + end + + it 'extracts transport_meta without attempts from Html2rss::Error on the chain', :aggregate_failures do + error = Html2rss::RequestService::RequestTimedOut.new('queued', timeout_phase: 'queue') + error.transport_meta = { request_id: 'req-direct', render_ms: 99, strategy_used: 'botasaurus' } + + expect(described_class::Diagnostics.from_error(error)).to have_attributes( + request_id: 'req-direct', render_ms: 99, strategy_used: 'botasaurus', timeout_phase: 'queue' + ) + expect(described_class::Diagnostics.from_error(error).strategy_attempts).to eq([]) + end end describe '.error_chain' do diff --git a/spec/html2rss/web/feeds/responder_spec.rb b/spec/html2rss/web/feeds/responder_spec.rb index b9a1f98c..fcc9efd3 100644 --- a/spec/html2rss/web/feeds/responder_spec.rb +++ b/spec/html2rss/web/feeds/responder_spec.rb @@ -218,6 +218,35 @@ expect(body).to eq(Html2rss::Web::ErrorClassifier::EXTRACTION_EMPTY_MESSAGE) end + it 'emits blocked surface empty with cacheable telemetry', :aggregate_failures do # rubocop:disable RSpec/ExampleLength + blocked = Html2rss::Web::Feeds::Contracts::RenderResult.new( + status: :empty, + payload: Html2rss::Web::Feeds::Contracts::RenderPayload.new( + feed: nil, + site_title: 'https://example.com', + url: 'https://example.com' + ), + ttl_seconds: 600, + cache_key: 'feed_result:blocked', + decision: Html2rss::Web::ErrorClassifier::BLOCKED_SURFACE, + empty_reason: 'blocked_surface' + ) + allow(Html2rss::Web::Feeds::Service).to receive(:call).and_return(blocked) + + write_response + + expect(Html2rss::Web::Observability).to have_received(:emit).with( + event_name: 'feed.render', + outcome: 'failure', + details: include( + error_code: 'BLOCKED_SURFACE', + reason: 'blocked_surface', + cacheable: true + ), + level: :warn + ) + end + it 'emits empty extraction as a failure outcome' do # rubocop:disable RSpec/ExampleLength write_response diff --git a/spec/html2rss/web/feeds/service_spec.rb b/spec/html2rss/web/feeds/service_spec.rb index e855af67..d03915cb 100644 --- a/spec/html2rss/web/feeds/service_spec.rb +++ b/spec/html2rss/web/feeds/service_spec.rb @@ -316,6 +316,30 @@ def expected_payload expect(result.status).to eq(:empty) expect(result.empty_reason).to eq('blocked_surface') expect(result.decision).to eq(Html2rss::Web::ErrorClassifier::BLOCKED_SURFACE) + expect(result.cacheable?).to be(true) + + described_class.call(resolved_source) + described_class.call(resolved_source) + + expect(Html2rss).to have_received(:feed_result).once + end + end + + context 'when RequestTimedOut carries attempt diagnostics' do + let(:attempts) do + [{ strategy: :default, error_class: 'Html2rss::RequestService::RequestTimedOut', elapsed_ms: 500 }] + end + + before do + error = Html2rss::RequestService::RequestTimedOut.new('timed out', timeout_phase: 'work') + error.attempts = attempts + allow(Html2rss).to receive(:feed_result).with(resolved_source.generator_input).and_raise(error) + end + + it 'preserves strategy_attempts on the error RenderResult', :aggregate_failures do + expect(result.status).to eq(:error) + expect(result.decision).to eq(Html2rss::Web::ErrorClassifier::GATEWAY_TIMEOUT) + expect(result.diagnostics.strategy_attempts).to eq(attempts) end end