A Lista de Verificação de Revisão de Código que Construi Após 2.000 Pull Requests

Eu categorizei 2.147 comentários de PR que deixei em 18 meses. 34% eram sobre tratamento de erros. 22% eram sobre nomenclatura. Apenas 8% sobre desempenho. Isso não era o que eu esperava quando comecei a acompanhar minhas revisões. Eu pensei que estaria identificando problemas arquitetônicos e bugs complexos. Em vez disso, estive repetidamente apontando os mesmos problemas fundamentais: verificações de nulo ausentes, nomes de variáveis vagos e mensagens de erro que não diziam nada útil aos usuários. Depois de ver os mesmos problemas surgirem em incidentes de produção, percebi que esses não eram pequenos detalhes—eles eram a diferença entre sistemas que falham de forma elegante e sistemas que te acordam às 3 da manhã.

Por que a Maioria das Listas de Verificação de Revisão de Código Não Chegam ao Ponto

Todo mundo te diz para verificar o estilo do código, a cobertura de testes e a documentação. Isso é bom, mas não é onde a produção quebra. As listas de verificação que vejo sendo compartilhadas em blogs de engenharia se concentram no que é fácil de medir ao invés do que realmente importa. Elas vão te dizer para verificar se as funções têm menos de 50 linhas, mas não vão te dizer para verificar se o tratamento de erros realmente ajuda alguém a depurar o problema às 2 da manhã, quando os logs são seu único amigo.

As melhores revisões de código não apenas previnem bugs—elas previnem o tipo de bugs que desencadeiam incidentes. Uma verificação de nulo ausente não é apenas uma potencial falha; é um potencial evento de corrupção de dados quando essa falha acontece no meio de uma transação.

Comecei a acompanhar meus comentários de PR porque estava frustrado. Eu revisava código, aprovava, e então via o mesmo desenvolvedor cometer o mesmo erro no próximo PR. O feedback não estava pegando. Então, eu construí uma planilha. Cada comentário que deixei foi categorizado: tratamento de erros, nomenclatura, testes, segurança, desempenho, arquitetura ou outros. Depois de seis meses, eu tinha dados suficientes para ver padrões. Depois de dezoito meses, aqueles padrões se consolidaram em uma lista de verificação que realmente funcionava.

A parte surpreendente não foi apenas o que estava no topo da lista—foi o que não estava. Comentários sobre otimização de desempenho representaram apenas 8% das minhas revisões. Questões de segurança foram 6%. Essas são as coisas que nos obsessamos em discussões sobre arquitetura, mas, nas PRs do dia a dia, elas são raras. O que é comum? Desenvolvedores assumindo o caminho feliz, nomeando as coisas mal, e escrevendo mensagens de erro para si mesmos ao invés de para a pessoa que estará depurando à meia-noite.

Os Padrões de Tratamento de Erros que Continuam Quebrando a Produção

Aqui está minha lista numerada de problemas de tratamento de erros, classificados pela frequência com que causam incidentes reais:

1. Falhas silenciosas em trabalhos em segundo plano. O código captura uma exceção, a registra e continua. Parece razoável até você perceber que esse trabalho deveria enviar um e-mail de confirmação de pagamento. Agora seu cliente pensa que não foi cobrado, mas foi. Vejo esse padrão em 40% dos PRs de trabalhos em segundo plano. A correção é simples: se a operação é crítica, não capture a exceção—deixe-a subir e acionar seu alerta. Se não é crítica, documente por que está tudo bem falhar silenciosamente.
2. Mensagens de erro genéricas que ocultam o contexto. "Ocorreu um erro" não me diz nada. "Falha ao processar pagamento" é melhor, mas ainda inútil. "Falha ao cobrar o cartão terminando em 4242: fundos insuficientes (código de erro: cartão_rejeitado)" realmente ajuda. Eu marco isso em cerca de 30% dos PRs. O teste que uso: se você visse esse erro nos logs de produção às 3 da manhã, conseguiria diagnosticá-lo sem adicionar mais logs e fazer uma nova implantação?
3. Capturando Exception em vez de exceções específicas. Isso é controverso porque alguns guias de estilo recomendam, mas eu vi isso esconder bugs muitas vezes. Quando você captura Exception, você também captura NullPointerException, IllegalStateException e todas as outras exceções que indicam erro do programador em vez de condições de tempo de execução. Capture o que você espera lidar. Deixe os erros do programador causar falhas—é assim que você os encontra.
4. Não validando dados externos na fronteira. Respostas da API, entrada do usuário, conteúdos de arquivos—se vem de fora do seu processo, valide imediatamente. Eu vejo desenvolvedores validando na camada de lógica de negócios, o que significa que dados inválidos já se propagaram através de várias funções. Nesse ponto, você está depurando por que um valor nulo entrou em seu banco de dados. Valide na fronteira, falhe rapidamente, retorne erros claros.
5. Lógica de repetição sem aumento exponencial. O serviço está fora do ar, então você tenta novamente imediatamente. Ele ainda está fora do ar, então você tenta novamente imediatamente. Parabéns, você acabou de transformar uma degradação de serviço em uma queda completa ao forçá-lo com tentativas. Eu vejo isso em 15% dos PRs que adicionam lógica de repetição. Sempre use aumento exponencial com jitter. Sempre tenha uma contagem máxima de tentativas. Sempre considere se tentar novamente realmente faz sentido para essa operação.
6. Não lidando com falhas parciais em operações em lote. Você está processando 1.000 registros. O registro 500 falha. O que acontece com os registros 501-1000? Na maioria dos códigos que reviso, eles nunca são processados. O lote falha, é retomado, falha novamente no registro 500, e você fica preso. Lide explicitamente com falhas parciais: acompanhe o que teve sucesso, o que falhou e por quê. Faça suas operações em lote retencíveis.
7. Assumindo que as transações de banco de dados sempre terão sucesso. Você inicia uma transação, faz algum trabalho e comete. Mas e se a confirmação falhar? E se você perder a conexão com o banco de dados no meio da transação? Vejo código que não lida com isso em 25% dos PRs que tocam no código do banco de dados. O resultado: o estado da sua aplicação e o estado do banco de dados divergem, e você passa horas depurando por que os dados não correspondem ao que os logs dizem que aconteceu.

O Dia em que um Nome de Variável Causou um Incidente de $50.000

Era uma manhã de terça-feira quando recebi a mensagem no Slack: "Acabamos de reembolsar $50,000 para os clientes errados." Eu abri o canal de incidentes e comecei a ler. Um desenvolvedor havia enviado uma correção na noite anterior para um bug em nosso sistema de processamento de reembolso. A correção era uma linha. O PR foi aprovado por dois engenheiros seniores. Os testes passaram. Tudo parecia bem.

O bug estava em um nome de variável. O código original tinha uma variável chamada `refundAmount` que representava o valor a ser reembolsado em centavos. O desenvolvedor adicionou uma nova variável chamada `refundAmount` que representava o valor em dólares. Eles esqueceram de renomear a variável original. O código compilou bem—ambas eram inteiros. Os testes passaram porque os dados de teste usaram valores em que centavos e dólares estavam próximos o suficiente para que as asserções não pegassem isso.

Na produção, processamos 200 reembolsos naquela manhã. Metade deles eram para o valor errado. Um reembolso de $10,00 se tornou um reembolso de $1.000. Um reembolso de $5,00 se tornou um reembolso de $500. Quando alguém percebeu, já havíamos pago em excesso $50.000. Tivemos que revisar manualmente cada reembolso, contatar os clientes e, em alguns casos, pedir o dinheiro de volta. Levou três dias para resolver.

A causa raiz não foi o erro do desenvolvedor—todo mundo comete erros. Foi que tínhamos duas variáveis com o mesmo nome representando diferentes unidades no mesmo escopo. A revisão de código não pegou isso porque os revisores estavam focados na lógica, não na nomenclatura. Após esse incidente, adicionei uma regra à minha lista de verificação: se uma variável representa uma quantidade com unidades, a unidade deve estar no nome. Não `amount`, mas `amountCents` ou `amountDollars`. Não `duration`, mas `durationSeconds` ou `durationMilliseconds`.

Isso parece pedante u