Skip to content

ref(rust): Deny unwrap/expect in message processors#7810

Open
onewland wants to merge 4 commits intomasterfrom
ref/forbid-rust-panics
Open

ref(rust): Deny unwrap/expect in message processors#7810
onewland wants to merge 4 commits intomasterfrom
ref/forbid-rust-panics

Conversation

@onewland
Copy link
Contributor

@onewland onewland commented Mar 11, 2026

Deny clippy::unwrap_used and clippy::expect_used in the processors module, where message processing happens. This is where bad input data is most likely to trigger panics — like INC-2060 where .unwrap() on a missing field caused a panic.

All existing production unwraps in processors are fixed:

  • eap_items: Handle missing received field gracefully (the exact INC-2060 bug)
  • eap_items: Return Result from read_item_id instead of panicking
  • errors: Use unwrap_or_else for UUID fallback
  • generic_metrics / release_health_metrics: Use filter_map for tag key parsing

Test code is exempted via #[allow] on test modules.

Refs EAP-439, EAP-438

@linear-code
Copy link

linear-code bot commented Mar 11, 2026

@onewland onewland changed the title ref(rust): Add clippy lints to deny panic macros in Rust consumers ref(rust): Deny unwrap/expect in message processors Mar 11, 2026
@@ -1,4 +1,6 @@
pub mod eap_items;
#![deny(clippy::unwrap_used, clippy::expect_used)]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the important change

onewland and others added 3 commits March 11, 2026 09:04
Configure clippy to deny `panic!`, `todo!`, `unimplemented!`, and
`unreachable!` macros, preventing future panics in production code.
Replace existing panic calls with proper error handling:

- config.rs: Return serde deserialization error instead of panicking
- factory_v2.rs: Log error and exit instead of panicking
- processor.rs: Log error and exit instead of panicking on schema error
- python.rs: Return InvalidMessage error instead of panicking
- writer_v2.rs: Replace unreachable! with anyhow::bail!

Refs EAP-439

Co-Authored-By: Claude <noreply@anthropic.com>
Add `clippy::unwrap_used` and `clippy::expect_used` as deny lints
scoped to the processors module, where message processing happens.
This would have caught the INC-2060 panic on a missing field.

Fix all existing production unwraps in processors:
- eap_items: Handle missing `received` field gracefully
- eap_items: Return Result from read_item_id instead of panicking
- errors: Use unwrap_or_else for UUID fallback
- generic_metrics: Use filter_map for tag key parsing
- release_health_metrics: Use filter_map for tag key parsing

Test code is exempted via #[allow] on test modules.

Refs EAP-439

Co-Authored-By: Claude <noreply@anthropic.com>
@onewland onewland force-pushed the ref/forbid-rust-panics branch from be78b85 to e639ac8 Compare March 11, 2026 16:05
@onewland onewland marked this pull request as ready for review March 11, 2026 16:09
@onewland onewland requested review from a team as code owners March 11, 2026 16:09
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Fix split_at panic in read_item_id by using .get() with proper error
handling. Fix filter_map silently dropping unparseable tag keys which
could create mismatched parallel arrays — use map+collect with error
propagation instead.

Co-Authored-By: Claude <noreply@anthropic.com>
@onewland
Copy link
Contributor Author

Also fixes EAP-438

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant