From aed7ad038996c822848b0b705b3f0918c5148e69 Mon Sep 17 00:00:00 2001 From: Hirley Date: Mon, 31 Aug 2026 20:46:44 -0300 Subject: [PATCH] =?UTF-8?q?Reagenda=20entrega=20de=20webhook=20quando=20a?= =?UTF-8?q?=20falha=20=C3=A9=20tempor=C3=A1ria?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fecha #76. WebhookDeliveryJob chamava WebhookDelivery.entregar e descartava o retorno. Como o serviço capturava tudo, logava e devolvia false, o ActiveJob dava a entrega por concluída em qualquer cenário: endpoint fora do ar, timeout, 5xx — tudo virava um aviso no log que ninguém reprocessava. O retorno de #entregar passa a ser a decisão: * true — entregue (2xx); * false — recusa definitiva (assinatura pausada ou excluída, URL reprovada na checagem de SSRF, 4xx, erro de TLS): o job encerra sem gastar tentativa, porque repetir não mudaria a resposta; * FalhaTemporaria — erro de rede, 429 ou 5xx: única exceção que escapa de propósito, e é o que o retry_on do job observa. Cinco tentativas com backoff polinomial. O bloco do retry_on registra a desistência em nível error — sem ele, a entrega perdida voltaria a sumir em silêncio, só que na quinta falha em vez da primeira. Duas classificações que não são óbvias, e por isso estão comentadas no código: erro de TLS não repete (certificado errado é configuração do outro lado, e repetir mantém uma assinatura quebrada parecendo viva), e erro inesperado também não (é bug nosso; repetir não conserta e atrasa o diagnóstico). O rescue de FalhaTemporaria precisa vir antes do rescue de StandardError: como ela herda dele, sem essa cláusula a exceção seria engolida e viraria de novo o false silencioso que esta mudança elimina. Junto vem config.active_job.queue_adapter = :test no ambiente de teste. Sem isso valia o :async, que roda o job de verdade numa thread de fundo durante o teste — contra o mesmo banco, dentro de uma transação que o RSpec desfaz, e com chance de tentar rede. Hoje quase nunca disparava porque os specs dublam perform_later, mas isso era acidente, não desenho; e é o adapter :test que permite verificar o retry sem esperar backoff real. Co-Authored-By: Claude Opus 5 --- README.md | 12 ++++- app/jobs/webhook_delivery_job.rb | 35 ++++++++++--- app/services/webhook_delivery.rb | 68 +++++++++++++++++++++++--- config/environments/test.rb | 9 ++++ spec/jobs/webhook_delivery_job_spec.rb | 30 ++++++++++++ spec/services/webhook_delivery_spec.rb | 43 ++++++++++++++-- 6 files changed, 179 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 50c9d11..9a1992d 100644 --- a/README.md +++ b/README.md @@ -260,7 +260,17 @@ Além do Telegram, o **admin** pode cadastrar webhooks genéricos em `/webhooks` - `WebhookSubscription` (`app/models/webhook_subscription.rb`) guarda a URL e os eventos escolhidos (array nativo do Postgres). Um webhook pode ser pausado (`active: false`) sem precisar excluir o cadastro. - `WebhookDispatcher` (`app/services/webhook_dispatcher.rb`) encontra as assinaturas ativas que escutam o evento e enfileira `WebhookDeliveryJob` pra cada uma — em background (adapter `:async` padrão do Rails; este projeto não tem Sidekiq/Solid Queue configurado), pra não travar a request original no tempo de resposta de um serviço de terceiro. -- `WebhookDelivery` (`app/services/webhook_delivery.rb`) faz o `POST` de fato, com timeout curto (5s). Um endpoint de terceiro fora do ar, lento ou respondendo erro não derruba nada — só loga e segue; não há retry. +- `WebhookDelivery` (`app/services/webhook_delivery.rb`) faz o `POST` de fato, com timeout curto (5s). O retorno dele é o que decide se o job tenta de novo, e a distinção é deliberada: + + | Situação | Resultado | O job… | + |---|---|---| + | `2xx` | `true` | encerra | + | assinatura pausada ou excluída, URL reprovada na checagem de SSRF, resposta `4xx`, erro de TLS | `false` | encerra **sem gastar tentativa** — repetir não mudaria a resposta | + | erro de rede (conexão recusada, timeout, DNS), `429`, `5xx` | levanta `FalhaTemporaria` | reagenda, com backoff, até 5 tentativas | + + Esgotadas as tentativas, `WebhookDeliveryJob` registra a desistência no log em nível `error` — a entrega perdida deixa rastro em vez de sumir. Um erro inesperado (bug nosso, não instabilidade do outro lado) também não consome tentativas: é logado como `error` e encerra, porque repetir cinco vezes não conserta e só atrasa o diagnóstico. + + Vale a ressalva: como o adapter ainda é o `:async` em memória, as tentativas reagendadas também se perdem num restart do processo. O retry cobre a instabilidade curta, que é a maioria; durabilidade de verdade depende de trocar o backend da fila. - **Proteção contra SSRF**: `PublicHttpTarget` (`app/services/public_http_target.rb`) resolve o host e recusa endereços de rede privada/local (`127.0.0.0/8`, `10.0.0.0/8`, `172.16.0.0/12`, `192.168.0.0/16`, `169.254.0.0/16` e as faixas IPv6 equivalentes), pra que um webhook não vire um jeito de fazer a aplicação bater num serviço interno da própria rede. A checagem roda **duas vezes, de propósito**: - no **cadastro/edição** (validação de `WebhookSubscription`), só pra dar o erro no formulário enquanto o admin ainda está na tela; - de novo na **hora da entrega** (`WebhookDelivery`), imediatamente antes de conectar — e é essa que protege de fato. Sozinha, a validação do cadastro não segura *DNS rebinding*: como a entrega acontece quando um evento dispara (possivelmente dias depois), bastaria cadastrar um host que resolve pra um IP público e trocar o registro DNS com calma. O IP verificado na entrega é passado direto pro `Net::HTTP` (`ipaddr:`), então o endereço checado é exatamente o endereço conectado — sem intervalo entre a checagem e o uso. O host original continua valendo pro cabeçalho `Host`, SNI e validação do certificado TLS. diff --git a/app/jobs/webhook_delivery_job.rb b/app/jobs/webhook_delivery_job.rb index 9d90d90..413012f 100644 --- a/app/jobs/webhook_delivery_job.rb +++ b/app/jobs/webhook_delivery_job.rb @@ -1,14 +1,37 @@ # frozen_string_literal: true -# Roda em background (adapter padrão do Rails, :async — não há -# Sidekiq/Solid Queue configurado neste projeto; jobs em andamento se -# perdem se o processo reiniciar, aceitável pro porte atual) pra não -# travar a request original no tempo de resposta de um serviço de -# terceiro. Ver WebhookDispatcher (quem enfileira) e WebhookDelivery -# (quem sabe fazer o POST de fato). +# Roda em background pra não travar a request original no tempo de +# resposta de um serviço de terceiro. Ver WebhookDispatcher (quem +# enfileira) e WebhookDelivery (quem sabe fazer o POST de fato). +# +# O adapter ainda é o padrão do Rails (:async), que guarda a fila na +# memória do processo web — jobs pendentes se perdem num restart, e as +# tentativas reagendadas abaixo também. Trocar por um backend persistente +# é assunto de outra issue; o retry aqui é útil de qualquer forma, porque +# a maioria das instabilidades de rede passa em segundos, muito antes de +# um deploy. class WebhookDeliveryJob < ApplicationJob queue_as :default + TENTATIVAS = 5 + + # Só WebhookDelivery::FalhaTemporaria reagenda. Uma recusa definitiva + # (assinatura pausada, URL reprovada na checagem de SSRF, 4xx) volta + # como `false` e encerra sem gastar tentativa — ver o contrato de + # retorno documentado em WebhookDelivery#entregar. + # + # O bloco roda quando as tentativas acabam. Sem ele, a desistência + # seria silenciosa, que é exatamente o problema que este retry veio + # resolver — só que adiado para a quinta falha em vez da primeira. + retry_on WebhookDelivery::FalhaTemporaria, wait: :polynomially_longer, attempts: TENTATIVAS do |job, erro| + subscription_id, event = job.arguments + + Rails.logger.error( + "[WebhookDeliveryJob] desisti de entregar \"#{event}\" pra assinatura #{subscription_id} " \ + "depois de #{TENTATIVAS} tentativas: #{erro.message}" + ) + end + def perform(subscription_id, event, payload) subscription = WebhookSubscription.find_by(id: subscription_id) WebhookDelivery.entregar(subscription, event, payload) diff --git a/app/services/webhook_delivery.rb b/app/services/webhook_delivery.rb index 3206445..5ab9abf 100644 --- a/app/services/webhook_delivery.rb +++ b/app/services/webhook_delivery.rb @@ -10,6 +10,22 @@ # evita depender de uma gem de mock de rede — o Gemfile não tem # WebMock/VCR — e evita qualquer chamada de rede real nos testes). class WebhookDelivery + # Única exceção que escapa daqui de propósito. É o sinal que faz + # WebhookDeliveryJob reagendar a entrega (ver o retry_on lá). + class FalhaTemporaria < StandardError; end + + # Erros de rede que costumam passar sozinhos: o endpoint pode estar + # reiniciando, o DNS oscilando, a rota caindo por um instante. + # + # Erro de TLS fica de fora de propósito. Certificado inválido, expirado + # ou com nome errado é configuração do outro lado — repetir cinco vezes + # não conserta, só adia o diagnóstico e mantém uma assinatura quebrada + # parecendo viva. + ERROS_TEMPORARIOS = [ + Errno::ECONNREFUSED, Errno::ECONNRESET, Errno::EHOSTUNREACH, Errno::ENETUNREACH, + Net::OpenTimeout, Net::ReadTimeout, Timeout::Error, EOFError, SocketError + ].freeze + # +ipaddr+ é o endereço já verificado por PublicHttpTarget. Passar ele # pro Net::HTTP faz a conexão ir direto nesse IP, sem resolver o host de # novo — é o que impede que o DNS mude entre a checagem e a conexão @@ -33,11 +49,18 @@ def initialize(transport: DEFAULT_TRANSPORT) @transport = transport end - # Não levanta exceção pra fora — um endpoint de terceiro fora do ar, - # lento ou respondendo erro não pode derrubar o job (nem, se algum dia a - # entrega virar síncrona, a request que originou o evento). Loga e - # retorna false; não há retry (fora do escopo deste MVP — ver issue de - # webhooks de saída no board). + # O retorno é o que decide se o job tenta de novo: + # + # * +true+ — entregue (2xx); + # * +false+ — recusa definitiva: assinatura inexistente ou pausada, + # URL reprovada na checagem de SSRF, ou resposta 4xx. Tentar de novo + # não mudaria nada, então o job encerra sem gastar tentativa; + # * +FalhaTemporaria+ — erro de rede, 429 ou 5xx. + # + # Antes, TODA falha virava +false+ e o ActiveJob dava a entrega por + # concluída: um endpoint fora do ar sumia com um aviso no log e ninguém + # reprocessava. A distinção acima existe pra que só o caso que vale a + # pena repetir chegue ao retry_on. def entregar(subscription, event, payload) return false unless subscription&.active? @@ -53,15 +76,44 @@ def entregar(subscription, event, payload) end body = { event: event, occurred_at: Time.current.iso8601, data: payload }.to_json - response = @transport.call(alvo.uri, body, alvo.ip) - response.is_a?(Net::HTTPSuccess) + entregue?(subscription, event, @transport.call(alvo.uri, body, alvo.ip)) + rescue FalhaTemporaria + # Precisa vir ANTES do rescue de StandardError: FalhaTemporaria herda + # dele, e sem esta cláusula a exceção levantada por #entregue? seria + # engolida logo abaixo — virando de novo o `false` silencioso que + # esta classe deixou de produzir. + raise + rescue *ERROS_TEMPORARIOS => e + raise FalhaTemporaria, "#{subscription.url}: #{e.class} #{e.message}" rescue StandardError => e - Rails.logger.warn("[WebhookDelivery] falha ao entregar \"#{event}\" pra #{subscription&.url}: #{e.class} #{e.message}") + # Erro inesperado aqui é bug nosso, não instabilidade do outro lado. + # Repetir não conserta e só atrasa o diagnóstico — loga alto e + # encerra, sem consumir tentativa. + Rails.logger.error( + "[WebhookDelivery] erro inesperado ao entregar \"#{event}\" pra #{subscription&.url}: #{e.class} #{e.message}" + ) false end private + def entregue?(subscription, event, response) + return true if response.is_a?(Net::HTTPSuccess) + raise FalhaTemporaria, "#{subscription.url} respondeu #{response.class}" if temporaria?(response) + + Rails.logger.warn( + "[WebhookDelivery] entrega de \"#{event}\" pra #{subscription.url} recusada com #{response.class}" + ) + false + end + + # 5xx é problema do servidor do outro lado; 429 é ele pedindo pra + # esperar. Os dois merecem outra tentativa. Um 4xx qualquer, não: o + # payload ou a rota é que estão errados, e repetir só gera ruído. + def temporaria?(response) + response.is_a?(Net::HTTPServerError) || response.is_a?(Net::HTTPTooManyRequests) + end + # Só registra o motivo — quem chama é que decide desistir da entrega. O # `return false` fica lá em cima, visível junto do `unless`, em vez de # escondido no fim de um método cujo nome não deixa claro que ele diff --git a/config/environments/test.rb b/config/environments/test.rb index 27d5570..0bf54c8 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -47,6 +47,15 @@ config.active_storage.service = :test + # Sem isto valeria o default do Rails, :async, que roda o job de verdade + # numa thread de fundo durante o teste — contra o mesmo banco, dentro de + # uma transação que o RSpec vai desfazer, e (no caso de WebhookDelivery) + # com chance de tentar rede de verdade. Hoje isso quase nunca dispara, + # porque os specs dublam WebhookDeliveryJob.perform_later, mas é + # acidente e não desenho. O adapter :test também é o que permite + # verificar o retry de WebhookDeliveryJob sem esperar backoff real. + config.active_job.queue_adapter = :test + config.action_mailer.perform_caching = false config.action_mailer.delivery_method = :test # Necessário pra gerar URLs fora de uma requisição (o e-mail de diff --git a/spec/jobs/webhook_delivery_job_spec.rb b/spec/jobs/webhook_delivery_job_spec.rb index 5789e1a..0f91fc3 100644 --- a/spec/jobs/webhook_delivery_job_spec.rb +++ b/spec/jobs/webhook_delivery_job_spec.rb @@ -20,4 +20,34 @@ expect(WebhookDelivery).to have_received(:entregar).with(nil, 'demanda_criada', { id: 1 }) end end + + # Ver o retry_on em WebhookDeliveryJob e o contrato de retorno de + # WebhookDelivery#entregar. O que se verifica aqui é a decisão do job: + # reagendar só quando vale a pena, sem gastar tentativa numa recusa que + # não vai mudar de resposta. + describe 'retentativa' do + let(:subscription) { create(:webhook_subscription) } + let(:fila) { ActiveJob::Base.queue_adapter.enqueued_jobs } + + it 'reagenda quando a entrega falha por algo temporário' do + allow(WebhookDelivery).to receive(:entregar).and_raise(WebhookDelivery::FalhaTemporaria, 'endpoint fora do ar') + + expect { described_class.perform_now(subscription.id, 'demanda_criada', {}) } + .to change(fila, :size).by(1) + end + + it 'não reagenda quando a recusa é definitiva' do + allow(WebhookDelivery).to receive(:entregar).and_return(false) + + expect { described_class.perform_now(subscription.id, 'demanda_criada', {}) } + .not_to change(fila, :size) + end + + it 'não reagenda quando a entrega dá certo' do + allow(WebhookDelivery).to receive(:entregar).and_return(true) + + expect { described_class.perform_now(subscription.id, 'demanda_criada', {}) } + .not_to change(fila, :size) + end + end end diff --git a/spec/services/webhook_delivery_spec.rb b/spec/services/webhook_delivery_spec.rb index 39c8797..ad83aa1 100644 --- a/spec/services/webhook_delivery_spec.rb +++ b/spec/services/webhook_delivery_spec.rb @@ -59,10 +59,47 @@ expect(chamadas).to be_empty end - it 'não propaga exceção se o transporte falhar (ex.: endpoint fora do ar) e retorna false' do - delivery_com_erro = described_class.new(transport: ->(_uri, _body, _ip) { raise SocketError, 'falha de rede' }) + # Antes, toda falha virava `false` e o ActiveJob dava a entrega por + # concluída. O que separa "vale tentar de novo" de "não adianta" é o + # tipo de retorno, e é ele que WebhookDeliveryJob usa pra decidir — + # por isso cada caso abaixo verifica exceção OU booleano, nunca os dois. + describe 'classificação da falha' do + def delivery_que(&bloco) + described_class.new(transport: ->(_uri, _body, _ip) { bloco.call }) + end + + it 'levanta FalhaTemporaria quando o endpoint está fora do ar' do + expect { delivery_que { raise SocketError, 'falha de rede' }.entregar(subscription, 'demanda_criada', {}) } + .to raise_error(WebhookDelivery::FalhaTemporaria, /SocketError/) + end + + it 'levanta FalhaTemporaria quando a conexão estoura o tempo' do + expect { delivery_que { raise Net::ReadTimeout }.entregar(subscription, 'demanda_criada', {}) } + .to raise_error(WebhookDelivery::FalhaTemporaria) + end + + it 'levanta FalhaTemporaria quando o endpoint responde 5xx' do + expect { delivery_que { Net::HTTPServiceUnavailable.allocate }.entregar(subscription, 'demanda_criada', {}) } + .to raise_error(WebhookDelivery::FalhaTemporaria) + end + + it 'levanta FalhaTemporaria quando o endpoint pede pra esperar (429)' do + expect { delivery_que { Net::HTTPTooManyRequests.allocate }.entregar(subscription, 'demanda_criada', {}) } + .to raise_error(WebhookDelivery::FalhaTemporaria) + end - expect(delivery_com_erro.entregar(subscription, 'demanda_criada', {})).to be false + # Certificado inválido é configuração do outro lado, não + # instabilidade: repetir não conserta e mantém uma assinatura + # quebrada parecendo viva. Ver ERROS_TEMPORARIOS. + it 'não repete um erro de TLS' do + expect(delivery_que { raise OpenSSL::SSL::SSLError, 'certificado expirado' } + .entregar(subscription, 'demanda_criada', {})).to be false + end + + it 'não repete um erro inesperado, que seria bug nosso' do + expect(delivery_que { raise ArgumentError, 'payload malformado' } + .entregar(subscription, 'demanda_criada', {})).to be false + end end # Ver PublicHttpTarget: a validação do cadastro sozinha não segura DNS