Repository navigation
PHPAY-174: feat: fall back only when nothing provably happened - #183
Merged
Merged
Conversation
FallbackGateway tries a list of gateways in order, and the question it asks is never 'did it fail?' - it is 'did it provably not happen?'. Handing the same charge to another gateway after an ambiguous failure creates a second charge, and that is worse than a repeated retry: a retry can be deduplicated by an idempotency key, and two different PSPs share nothing that could deduplicate them. The answer comes from Http\Delivery, extracted from RetryPolicy so the two policies consult one copy. A second copy would drift, and the drift would show up as somebody paying twice. supportedMethods() returns the union, not the intersection, and that is not the mistake declaring the union of capabilities would be: a capability is asked once against this object, while a method is chosen per charge, and createCharge() filters the list before trying anything. Without that filter a Pix charge would reach a gateway that cannot do Pix, after the one that could had already been given up on. Testing this found the Cielo dropping a due date in silence on the portable path. It now refuses it, and the billet left its supportedMethods() until the Cielo's own field name is confirmed - guessing it is what this project refuses to do with the Rede webhook envelope.
A timeout with the connection already open is a ConnectException in Guzzle 7 and a NetworkTimeoutException in Guzzle 8, so the same test cannot hold in both: on 8 the class alone already means nothing was sent, and the fallback is right to take it. Each case now skips on the major where it does not exist, with the reason, the way RetryPolicyTest already does - and the Guzzle 8 side gets its own test instead of a gap. Verified by installing Guzzle 8 locally and running the whole suite and PHPStan against it, not by reasoning about it.
Collaborator
Author
Correção do CI: dois cenários que pertencem a uma major sóOs jobs Guzzle 8 e PHP 8.4 - highest reprovaram num teste só — O motivo é o que o Cada caso agora leva Verifiquei instalando a Guzzle 8 de verdade, não raciocinando sobre ela:
Depois restaurei a 7 e conferi que o |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #174
A regra
A pergunta nunca é "falhou?". É "ficou provado que não aconteceu?" — porque
entregar a mesma cobrança a outro gateway depois de uma falha ambígua cria uma
segunda cobrança, e aqui é pior que um retry repetido: retry o gateway
dedupe por chave de idempotência, e entre dois PSPs não existe chave de
idempotência.
Cada linha tem teste.
Uma cópia só da classificação
A issue pedia para não reimplementar o
neverSent(). Ele saiu doRetryPolicyparaPHPay\Http\Delivery, e as duas políticas consultam de lá.O parágrafo do docblock que explicava as duas majors do Guzzle foi junto, porque
quem responde a pergunta agora é o
Delivery.União de meios, interseção de capacidades
supportedMethods()devolve a união — e isso não é o erro que declarara união de capacidades seria:
declara só cobrança e cobrança portável, nunca clientes, webhooks ou estornos
dos gateways de dentro;
createCharge()filtra a lista antesde tentar qualquer coisa.
Então uma cadeia de Asaas e Rede realmente faz Pix e crédito — só nunca no
mesmo gateway. Sem essa filtragem, uma cobrança Pix chegaria a quem não faz
Pix, depois de já ter desistido de quem fazia. Era a armadilha que a issue
descrevia.
O que os testes ensinaram sobre as camadas
Os primeiros testes de erro de transporte falharam, e o motivo é bom: o retry
roda dentro do gateway e o fallback fora dele. Um erro que provadamente não
saiu é repetido três vezes no mesmo gateway antes de o próximo entrar — então a
fila precisa de três respostas, não uma. Virou teste nomeado.
Uma cadeia pode ser inútil sem avisar
Montei os testes com Asaas + Woovi e eles não funcionavam: o Asaas exige
vencimento no Pix e a Woovi o recusa, então aquela dupla nunca tem as duas
como candidatas para a mesma cobrança. O fallback funciona — sempre cai —, mas
a lista não dá a redundância que aparenta. Está documentado, porque é o tipo de
coisa que só se descobre em produção.
Um defeito da fase 3 que este trabalho encontrou
Ao procurar uma cadeia que servisse, descobri que a Cielo engolia o
dueDateem silêncio: o corpo saía sem nenhum traço da data. Corrigi aquiporque é código que eu mesmo escrevi há pouco, e porque silêncio em campo
monetário é exatamente o que este projeto recusa.
CieloSaleRequest::fromIntent()agora recusadueDate;Method::BOLETOsaiu dosupportedMethods()da Cielo, porqueChargeIntent::boleto()exige vencimento e o nome do campo na API dela eunão confirmei — procurei na documentação e numa busca. Chutar
ExpirationDateseria o que o projeto recusa com o envelope da Rede.É redução do que a fase 3 entregou, e é correção: engolir a data é pior que não
suportar.
Observabilidade
Sem isso, um primário fora do ar fica invisível.
Fora de escopo, e dito na documentação
charge()é do primário e não tem fallback — devolve recurso com estado,e qual gateway atende só se decide na chamada terminal, dentro de um objeto
que o decorator já não segura. Recusá-lo seria declarar capacidade e lançar
de dentro;
núcleo é sem estado de propósito;
createCharge()cria o cliente antes da cobrança, então cair deixaum cliente órfão lá. Registro sem dinheiro, bem menos grave que cobrança
dupla, e está documentado.
Portão
docs/fallback.mdnovo, ligado no índice