fix: nomear o motivo real do descarte de evento e tornar tipo sem rota visivel - #2
Merged
Merged
Conversation
…ed types
Three defects in the same code path, all of them silent.
1. route_event/1 inferred the reason from an empty list. resolve_topics/1
returns [] for two unrelated reasons, and both logged "Event missing
required fields" while printing an event that had every required field in
it. Whoever debugged that in production was pointed at the wrong cause by a
message the event itself contradicted. The two cases are now told apart, and
the missing-field case names which field.
2. A `type` that decoded as a number reached String.starts_with?/2 and raised
FunctionClauseError inside handle_info, taking the subscriber down. The
moduledoc says a malformed event must not do that. resolve_topics/1 now
guards on binaries, so a non-binary field is classified as missing and
logged, like any other malformed envelope.
3. The unrouted branch logged at :debug. Production runs at :info and up, so an
event type nobody routes was invisible - which is how nine published types
went unnoticed. It is a warning now: it means either a new type the
publisher started sending or a typo, and both need a human.
The payload is no longer logged in any of these paths. It can carry user data,
and logs are retained and aggregated far more widely than the Redis channel
they came from.
- route_event/1 delegates to log_drop/1; broadcast extracted
- resolve_topics/1 no longer logs, and its docstring stops claiming a purity
it never had (specific_topics/4 still warns, deliberately)
- @supported_versions and @required_fields moved above first use
- 5 tests covering both reasons, the non-binary field, the payload not
leaking, and the warning level
- @moduletag :capture_log so the new warning does not pollute the suite
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
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
1 participant
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.
O que esta PR faz?
Fecha as duas stories de observabilidade do PRD de contrato e entrega (US-03 e US-08), e um terceiro
defeito que apareceu durante a implementação.
O
resolve_topics/1devolve[]por dois motivos que não têm relação entre si, e oroute_event/1inferia o motivo a partir da lista vazia. Os dois casos logavam
"Event missing required fields"imprimindo, ao lado, um evento que tinha todos os campos obrigatórios. Quem depurasse isso em
produção era apontado para a causa errada por uma mensagem que o próprio evento desmentia.
Tipo da PR
Bugfix.
O que esta PR adiciona, remove ou atualiza?
log_drop/1distingue versão não suportada de campo obrigatórioausente, e no segundo caso nomeia qual campo faltou.
typenão-string deixa de derrubar o subscriber. Umtypeque decodificava como númerochegava no
String.starts_with?/2e levantavaFunctionClauseErrordentro dohandle_info,matando o GenServer. O moduledoc diz que evento malformado não pode fazer isso. O
resolve_topics/1agora tem guard de binário, e campo não-binário é classificado como ausente.debugparawarning. Produção roda em:infopara cima, entãotipo que ninguém roteia era invisível - foi assim que nove tipos publicados passaram despercebidos.
é retido e agregado muito mais amplamente que o canal Redis de onde veio.
@moduletag :capture_log, senão o warning novo polui a saída dos testes que não olham log.Como testar?
Para ver a diferença na prática, comparado com a
main:version: 99Event missing required fields: %{...evento completo...}Rejected event: unsupported version=99 type="notification.created"user_idDropped event: missing or invalid field(s): user_idtype: 123Dropped event: missing or invalid field(s): typeroster.player_hired[debug], invisível em produção[warning] Unrouted event type=roster.player_hired org=...Resultado esperado
mix testem 60 testes, 0 falhas. Os seis portões verdes:format,compile --warnings-as-errors,credo --strict,sobelow --config --exitedialyzer.Nota de desenho
A docstring do
resolve_topics/1dizia "Pure function" e a função logava - não era verdade nemantes desta PR. Ela continua emitindo o warning de tipo não roteado, de propósito, porque o
typeeo
org_idsó estão em escopo ali. A docstring foi corrigida para dizer o que a função de fatogarante: nada de broadcast, e por isso a matriz de roteamento inteira é testável sem Redis.
Fora de escopo
Os tipos que hoje só chegam em
org_eventscontinuam sem destino assinável. Agora eles pelo menosaparecem no log. Dar destino a eles é a próxima story, e ela tem uma decisão de desenho aberta:
canais por família ou expor
org_events:*.