feat(udp-server): change connection ID error log level from ERROR to WARNING#1975
Merged
josecelano merged 3 commits intoJul 13, 2026
Conversation
…WARNING Connection cookie errors (expired/from-future) are caused by bad client traffic, not application errors. Logging them at ERROR level floods the logs, making it hard to identify real issues. Now inspects the error variant: - Connection cookie errors: logged at WARN - All other UDP errors: remain at ERROR Also updated the test helper log filter from ERROR to WARN so the captured test logs include WARN messages for assertions.
There was a problem hiding this comment.
Pull request overview
This PR reduces log noise in the UDP server by downgrading connection-cookie / connection-id validation errors from ERROR to WARN, while keeping all other UDP server errors at ERROR. This aligns the log severity with expected bad-client traffic patterns and preserves ERROR for real server-side failures.
Changes:
- Added error-variant detection so connection-cookie errors log at
WARNand all other errors remainERROR. - Updated the UDP server contract test to assert
WARNinstead ofERRORfor the affected scenario. - Adjusted the shared test log capture filter to include
WARNlogs.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
packages/udp-server/src/handlers/error.rs |
Conditional logging level based on whether the error is a connection-cookie error. |
packages/udp-server/tests/server/contract.rs |
Updated expected log level in the banning/invalid-connection-id test. |
packages/test-helpers/src/logging.rs |
Raised captured log verbosity to include WARN so tests can assert on warn-level output. |
docs/issues/open/1447-change-logging-threshold-connection-id-error.md |
Added a new issue spec documenting the rationale and plan for the change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Remove unused torrust_tracker_udp_core::self import - Add missing skill-link marker to issue spec
Member
Author
|
ACK b4fa8f9 |
josecelano
added a commit
that referenced
this pull request
Jul 15, 2026
…ssues/open to docs/issues/closed 174904d chore(ci): point to ADR instead of closed issue spec in deployment-packages.yaml (Jose Celano) 2e2178f docs(skill): document branch-already-exists edge case in cleanup-completed-issues skill (Jose Celano) 1bfd306 chore(issues): archive closed issue specs #1447, #1459, #1505, #1926, #1938, #1944, #1959, #1964, #1965, #1969 to docs/issues/closed (Jose Celano) Pull request description: Archives the following 10 issue specs that are closed on GitHub, moving them from `docs/issues/open/` to `docs/issues/closed/`: | Issue | Title | PR | |-------|-------|-----| | #1447 | Change logging threshold for connection ID error to WARNING | #1975 | | #1459 | Docker Security Overhaul | #1958 | | #1505 | Optimize peer IP list from swarm (implementation rejected) | #1949 | | #1926 | Define package versioning strategy | #1961 | | #1938 | REST API Contract-First Migration EPIC | #1960 (SI-1→5) | | #1944 | SI-6: Align REST API client | #1968 | | #1959 | SI-7: Review tests, align v1 namespace | #1963 | | #1964 | Rename number-of-downloads BTreeMap type alias | #1972 | | #1965 | SI-34: Consolidate duplicate HTTP types | #1974 | | #1969 | SI-8: Eliminate unwraps from REST API client | #1973 | ### Changes Made - All 10 specs verified `CLOSED` on GitHub with merged PRs - Files/directories moved via `git mv` to `docs/issues/closed/` - Frontmatter updated: `status: done`, `spec-path` → closed path, `last-updated-utc: 2026-07-15` - Workflow checkpoints updated and progress log entries added - Supplementary files in multi-file directories updated with new paths - Parent EPIC specs (#1669, #1938) updated: subissue status, spec paths, and table references - Fixed stale references in `.github/workflows/deployment-packages.yaml`, `docs/adrs/`, and `docs/issues/closed/` files - Pre-commit and pre-push hooks passed (linter, tests, doc-tests, nightly checks) ACKs for top commit: josecelano: ACK 174904d Tree-SHA512: c7e38354bb5898fc5125a0b68fd61a02ca273689593638eb3b955b9176038ef82c40fc98b0a2f722eb18d8d429d2f2dbb6f205eee3061211ab9906c5be46942b
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.
What
Changes the log level for UDP connection ID errors from
ERRORtoWARNING.Why
The UDP tracker receives a high volume of requests with invalid connection IDs from misconfigured or abusive peers (e.g. "cookie value is expired", "cookie value is from future"). These are not application errors — they are expected behaviour from bad client traffic. Logging them at
ERRORlevel floods the logs and makes it hard to identify real issues.The tracker already bans IPs that make too many such requests (tracked via the
udp_tracker_server_connection_id_errors_totalmetric and the ban service). AWARNINGlevel is still appropriate because the log remains the primary observability channel for connection ID issues.How
The
log_error()function inpackages/udp-server/src/handlers/error.rsnow inspects the error variant:tracing::warn!tracing::error!(unchanged)Changes
packages/udp-server/src/handlers/error.rsis_connection_cookie_error()helper; conditional log level inlog_error()packages/udp-server/tests/server/contract.rs"ERROR"→"WARN"packages/test-helpers/src/logging.rsLevelFilter::ERROR→LevelFilter::WARNso WARN messages are capturedVerification
linter all— all linters passcargo test --workspace— all 935 tests pass (0 failures)should_ban_the_client_ip_if_it_sends_more_than_10_requests_with_a_cookie_value_not_normalcorrectly assertsWARNlevel logsCloses #1447