Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]
name = "flowparser-sflow"
description = "Parser for sFlow v5 datagrams"
version = "0.1.1"
version = "0.2.0"
edition = "2024"
authors = ["michael.mileusnich@gmail.com"]
license = "MIT OR Apache-2.0"
Expand Down
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ Add to your `Cargo.toml`:

```toml
[dependencies]
flowparser-sflow = "0.1.0"
flowparser-sflow = "0.2.0"
```

### Basic Parsing
Expand Down Expand Up @@ -144,6 +144,8 @@ Datagram
| `AddressType` | IPv4 or IPv6 agent address |
| `ParseResult` | Contains parsed datagrams and optional error |
| `SflowError` | Error variants: Incomplete, UnsupportedVersion, ParseError, TooManySamples |
| `ParseContext` | Enum identifying the parsing phase where an error occurred |
| `ParseErrorKind` | Enum categorizing parse errors (InvalidAddressType, NomError) |

## Examples

Expand Down
32 changes: 32 additions & 0 deletions RELEASES.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,37 @@
# Releases

## 0.2.0

### Breaking Changes

- **`SflowError::Incomplete`**: `context` field changed from `String` to `ParseContext` enum; added `expected: Option<usize>` field
- **`SflowError::ParseError`**: `context` field changed from `String` to `ParseContext` enum; `kind` field changed from `String` to `ParseErrorKind` enum

### Added

- **34 new flow record types** (enterprise=0):
- MPLS & NAT (formats 1006–1012): `ExtendedMpls`, `ExtendedNat`, `ExtendedMplsTunnel`, `ExtendedMplsVc`, `ExtendedMplsFtn`, `ExtendedMplsLdpFec`, `ExtendedVlanTunnel`
- 802.11 wireless (formats 1013–1015): `Extended80211Payload`, `Extended80211Rx`, `Extended80211Tx`
- Tunnel (formats 1021–1030): `ExtendedL2TunnelEgress`, `ExtendedL2TunnelIngress`, `ExtendedIpv4TunnelEgress`, `ExtendedIpv4TunnelIngress`, `ExtendedIpv6TunnelEgress`, `ExtendedIpv6TunnelIngress`, `ExtendedDecapsulateEgress`, `ExtendedDecapsulateIngress`, `ExtendedVniEgress`, `ExtendedVniIngress`
- Queue, ACL, function, transit (formats 1036–1040): `ExtendedEgressQueue`, `ExtendedAcl`, `ExtendedFunction`, `ExtendedTransit`, `ExtendedQueue`
- Socket (formats 2100–2103): `ExtendedSocketIpv4`, `ExtendedSocketIpv6`, `ExtendedProxySocketIpv4`, `ExtendedProxySocketIpv6`
- Application & JVM (formats 2105, 2200, 2202, 2206, 2207): `JvmRuntime`, `MemcacheOperation`, `AppOperation`, `HttpRequest`, `ExtendedProxyRequest`
- **25 new counter record types** (enterprise=0):
- Core (formats 4, 6, 7, 10): `VgCounters`, `Ieee80211Counters`, `LagPortStats`, `Sfp`
- OpenFlow & radio (formats 1002, 1004, 1005): `RadioUtilization`, `OfPort`, `PortName`
- Host monitoring (formats 2000–2010): `HostDescr`, `HostAdapters`, `HostParent`, `HostCpu`, `HostMemory`, `HostDiskIo`, `HostNetIo`, `Mib2IpGroup`, `Mib2IcmpGroup`, `Mib2TcpGroup`, `Mib2UdpGroup`
- Application & JVM (formats 2106, 2201–2204, 2206): `JvmStatistics`, `HttpCounters`, `AppOperations`, `AppResources`, `MemcacheCounters`, `AppWorkers`
- `ParseContext` enum with 14 variants covering all parsing phases (e.g., `DatagramHeader`, `AgentAddress`, `FlowSample`)
- `ParseErrorKind` enum with `InvalidAddressType` and `NomError(nom::error::ErrorKind)` variants
- `expected: Option<usize>` field on `SflowError::Incomplete` for cases where the required byte count is known

### Improved

- Error paths no longer allocate on the heap (enums are `Copy` instead of `String`)
- Consumers can exhaustively match on error contexts and kinds
- Display output remains compatible with previous format
- Records previously parsed as `Unknown` (e.g., formats 2100, 2200, 1029, 1030) are now fully decoded

## 0.1.1

- Added crates.io badge to README
Expand Down
6 changes: 6 additions & 0 deletions examples/dump_hex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,9 @@ fn print_flow_record(idx: usize, rec: &FlowRecord) {
data.len()
);
}
other => {
println!(" Record[{idx}]: {other:?}");
}
}
}

Expand Down Expand Up @@ -273,6 +276,9 @@ fn print_counter_record(idx: usize, rec: &CounterRecord) {
data.len()
);
}
other => {
println!(" Record[{idx}]: {other:?}");
}
}
}

Expand Down
1 change: 1 addition & 0 deletions examples/sflow_pcap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ fn process_packet(data: &[u8], parser: &SflowParser, packet_count: &mut usize) {
datagram.sequence_number,
datagram.samples.len()
);
println!("{:?}", datagram);
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions openspec/changes/refactor-error-handling/.openspec.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-02-17
113 changes: 113 additions & 0 deletions openspec/changes/refactor-error-handling/design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
## Context

`SflowError` uses `String` for `context` and `kind` fields across 14 construction sites in `src/datagram.rs`, `src/samples/mod.rs`, and `src/lib.rs`. Every error path allocates on the heap. The set of possible values is finite and known at compile time, making enums a natural fit.

Current string values for `context` (in `Incomplete` and `ParseError`):
- Datagram header fields: `"datagram header"`, `"datagram header version"`, `"agent address"`, `"sub_agent_id"`, `"sequence_number"`, `"uptime"`, `"num_samples"`
- Sample parsing: `"sample data_format"`, `"sample length"`, `"sample data (need N bytes)"` (dynamic)
- Sample types: `"flow sample"`, `"counter sample"`, `"expanded flow sample"`, `"expanded counter sample"`

Current string values for `kind` (in `ParseError`):
- `"invalid address type"` (hardcoded in datagram.rs)
- nom `ErrorKind` debug names via `nom_error_kind()` helper (e.g., `"Eof"`, `"Switch"`)
- `"incomplete"` (from nom `Err::Incomplete`)

## Goals / Non-Goals

**Goals:**
- Replace `context: String` with a `ParseContext` enum
- Replace `kind: String` with a `ParseErrorKind` enum
- Maintain equivalent `Display` output for human-readable messages
- Keep `Serialize`/`Deserialize` working on all error types
- Eliminate heap allocations on error paths

**Non-Goals:**
- Restructuring the `SflowError` variants themselves (e.g., merging `Incomplete` and `ParseError`)
- Adding new error variants or error recovery mechanisms
- Changing the partial-parse behavior (`ParseResult` returning datagrams + optional error)

## Decisions

### 1. ParseContext enum design

**Choice**: A flat enum with one variant per parsing phase/field. The one dynamic case — `"sample data (need N bytes)"` — is handled by adding an `expected: Option<usize>` field to the `Incomplete` variant rather than putting dynamic data in the context enum.

```rust
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
pub enum ParseContext {
DatagramHeader,
DatagramHeaderVersion,
AgentAddress,
SubAgentId,
SequenceNumber,
Uptime,
NumSamples,
SampleDataFormat,
SampleLength,
SampleData,
FlowSample,
CounterSample,
ExpandedFlowSample,
ExpandedCounterSample,
}
```

The enum implements `Display` to produce the same human-readable strings used today (e.g., `ParseContext::SubAgentId` displays as `"sub_agent_id"`).

**Rationale**: A flat enum is simple, `Copy`, and exhaustively matchable. The `expected` field on `Incomplete` is a cleaner place for the byte count than embedding it in the context.

**Alternative considered**: Nested enums (e.g., `ParseContext::Datagram(DatagramField)`, `ParseContext::Sample(SampleField)`). Rejected — adds complexity for no benefit given the small number of variants.

### 2. ParseErrorKind enum design

**Choice**: An enum wrapping the two error categories: domain-specific errors and nom parsing errors.

```rust
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum ParseErrorKind {
InvalidAddressType,
NomError(nom::error::ErrorKind),
}
```

**Serde handling**: `nom::error::ErrorKind` does not implement `Serialize`/`Deserialize`. We implement custom serde for `ParseErrorKind` that serializes the nom variant as its `Debug` name (e.g., `"Eof"`, `"Switch"`), matching the current string output from `nom_error_kind()`.

**Rationale**: Wrapping `nom::error::ErrorKind` directly preserves all information without creating a parallel enum that duplicates nom's ~40 variants. Custom serde is a small, well-scoped addition.

**Alternative considered**: Creating our own `NomErrorKind` enum mirroring nom's variants. Rejected — maintenance burden, and we'd need to update it whenever nom adds variants.

### 3. Incomplete variant gains `expected` field

**Choice**: Add `expected: Option<usize>` to the `Incomplete` variant.

```rust
Incomplete {
available: usize,
expected: Option<usize>,
context: ParseContext,
},
```

`expected` is `Some(n)` only for the sample-data case where the required byte count is known. All other `Incomplete` sites set it to `None`.

**Rationale**: This replaces the dynamic `format!("sample data (need {sample_length} bytes)")` string without polluting the context enum. The `Display` impl includes the expected count when present.

### 4. nom_error_kind helper removal

**Choice**: Remove the `nom_error_kind()` function from `src/samples/mod.rs`. Replace call sites with direct construction of `ParseErrorKind::NomError(e.code)` / `ParseErrorKind::NomError(ErrorKind::Complete)` as appropriate.

**Rationale**: The helper existed solely to convert nom errors to strings. With `ParseErrorKind` wrapping `nom::error::ErrorKind` directly, no conversion is needed.

### 5. Display output compatibility

**Choice**: The `Display` impl produces messages equivalent to the current output. For example:
- `Incomplete { available: 3, expected: None, context: ParseContext::DatagramHeaderVersion }` → `"Incomplete data: only 3 bytes available (datagram header version)"`
- `Incomplete { available: 10, expected: Some(32), context: ParseContext::SampleData }` → `"Incomplete data: only 10 bytes available, expected 32 (sample data)"`

**Rationale**: Downstream consumers may parse or log these messages. Keeping the format stable reduces breakage surface — the breaking change is in the type system, not in human-readable output.

## Risks / Trade-offs

- **Breaking API change** → Semver minor/major bump required. Document migration in changelog. The set of context values is now fixed — consumers gain exhaustive matching but lose the ability to construct errors with arbitrary context strings.
- **nom version coupling** → `ParseErrorKind::NomError` wraps `nom::error::ErrorKind` directly, coupling to nom's API. → Mitigated: nom is already a direct dependency, and ErrorKind is stable across nom 7.x.
- **Custom serde for ParseErrorKind** → Small maintenance cost for the manual Serialize/Deserialize impl. → Mitigated: the impl is straightforward (serialize Debug name, deserialize from string match).
27 changes: 27 additions & 0 deletions openspec/changes/refactor-error-handling/proposal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
## Why

The `SflowError` enum uses `String` for `context` and `kind` fields in the `Incomplete` and `ParseError` variants. These are free-form strings scattered across the codebase (e.g., `"datagram header version".to_string()`, `"flow sample".to_string()`). Replacing them with descriptive enums improves type safety, eliminates heap allocations on error paths, enables exhaustive matching by consumers, and makes the error API self-documenting.

## What Changes

- **BREAKING**: Replace `context: String` in `Incomplete` with a new `ParseContext` enum covering all field/phase identifiers (e.g., `DatagramHeaderVersion`, `SubAgentId`, `SequenceNumber`, `SampleDataFormat`, etc.)
- **BREAKING**: Replace `context: String` in `ParseError` with the same `ParseContext` enum
- **BREAKING**: Replace `kind: String` in `ParseError` with a new `ParseErrorKind` enum (e.g., `InvalidAddressType`, `NomError(ErrorKind)`, `Incomplete`)
- Update all error construction sites in `src/datagram.rs`, `src/samples/mod.rs`, and `src/lib.rs`
- Update `Display` impl to produce equivalent human-readable messages
- Update tests in `src/tests.rs` and `tests/error_handling.rs`

## Capabilities

### New Capabilities
- `error-context-enum`: Introduces `ParseContext` and `ParseErrorKind` enums to replace free-form String fields in `SflowError`

### Modified Capabilities
- `public-api`: The `SflowError` variants `Incomplete` and `ParseError` change field types from `String` to enums (breaking change)

## Impact

- **Public API**: Breaking change to `SflowError` — any downstream code matching on `context` or `kind` fields will need to update
- **Code**: Changes across `src/error.rs` (enum definitions + Display), `src/datagram.rs`, `src/samples/mod.rs`, `src/lib.rs` (construction sites), and test files
- **Dependencies**: None added or removed. `nom::error::ErrorKind` is re-used inside `ParseErrorKind`
- **Performance**: Removes heap allocations (`String::to_string()`) on every error path — enums are `Copy`
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
## ADDED Requirements

### Requirement: ParseContext enum covers all parsing phases
The `ParseContext` enum SHALL include one variant for every distinct parsing phase or field where errors can originate. The variants SHALL be:
- `DatagramHeader` — top-level datagram header parsing
- `DatagramHeaderVersion` — version field in the datagram header
- `AgentAddress` — agent address field
- `SubAgentId` — sub-agent identifier field
- `SequenceNumber` — datagram sequence number field
- `Uptime` — agent uptime field
- `NumSamples` — sample count field
- `SampleDataFormat` — sample enterprise/format field
- `SampleLength` — sample length field
- `SampleData` — sample body data
- `FlowSample` — flow sample (format=1) parsing
- `CounterSample` — counter sample (format=2) parsing
- `ExpandedFlowSample` — expanded flow sample (format=3) parsing
- `ExpandedCounterSample` — expanded counter sample (format=4) parsing

#### Scenario: Exhaustive match on ParseContext
- **WHEN** a consumer matches on `ParseContext` with all 14 variants
- **THEN** the match SHALL be exhaustive with no wildcard arm needed

#### Scenario: ParseContext is Copy
- **WHEN** a `ParseContext` value is used
- **THEN** it SHALL be `Copy`, `Clone`, `Debug`, `PartialEq`, `Eq`, `Serialize`, and `Deserialize`

### Requirement: ParseContext Display produces human-readable names
Each `ParseContext` variant SHALL implement `Display` producing a lowercase, human-readable string matching the current string literals used in the codebase.

#### Scenario: Display output matches legacy strings
- **WHEN** `ParseContext::DatagramHeaderVersion` is formatted with `Display`
- **THEN** the output SHALL be `"datagram header version"`

#### Scenario: Display for sample types
- **WHEN** `ParseContext::ExpandedFlowSample` is formatted with `Display`
- **THEN** the output SHALL be `"expanded flow sample"`

### Requirement: ParseErrorKind enum covers all error categories
The `ParseErrorKind` enum SHALL include:
- `InvalidAddressType` — an unrecognized address type value was encountered
- `NomError(nom::error::ErrorKind)` — a nom parser error, wrapping the original error kind

#### Scenario: Domain-specific error
- **WHEN** an invalid address type is encountered during parsing
- **THEN** the error SHALL use `ParseErrorKind::InvalidAddressType`

#### Scenario: Nom parser failure
- **WHEN** a nom parser returns an `Err::Error` or `Err::Failure`
- **THEN** the error SHALL use `ParseErrorKind::NomError` wrapping the `nom::error::ErrorKind` from the error

#### Scenario: Nom incomplete error
- **WHEN** a nom parser returns `Err::Incomplete`
- **THEN** the error SHALL use `ParseErrorKind::NomError(nom::error::ErrorKind::Complete)`

### Requirement: ParseErrorKind derives required traits
`ParseErrorKind` SHALL derive `Debug`, `Clone`, `Copy`, `PartialEq`, and `Eq`. Since `nom::error::ErrorKind` does not implement `Serialize`/`Deserialize`, custom serde implementations SHALL be provided.

#### Scenario: Serialize NomError variant
- **WHEN** `ParseErrorKind::NomError(ErrorKind::Eof)` is serialized
- **THEN** the output SHALL be the string `"Eof"` (the Debug name of the nom variant)

#### Scenario: Deserialize NomError variant
- **WHEN** the string `"Eof"` is deserialized as a `ParseErrorKind`
- **THEN** the result SHALL be `ParseErrorKind::NomError(ErrorKind::Eof)`

#### Scenario: Serialize InvalidAddressType
- **WHEN** `ParseErrorKind::InvalidAddressType` is serialized
- **THEN** the output SHALL be the string `"InvalidAddressType"`
52 changes: 52 additions & 0 deletions openspec/changes/refactor-error-handling/specs/public-api/spec.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
## ADDED Requirements

### Requirement: Incomplete variant uses ParseContext and optional expected size
The `SflowError::Incomplete` variant SHALL have the following fields:
- `available: usize` — number of bytes available
- `expected: Option<usize>` — expected number of bytes when known, `None` otherwise
- `context: ParseContext` — the parsing phase where the error occurred

#### Scenario: Incomplete error for a header field
- **WHEN** the parser fails to read the `sub_agent_id` field due to insufficient bytes
- **THEN** the error SHALL be `SflowError::Incomplete { available: <n>, expected: None, context: ParseContext::SubAgentId }`

#### Scenario: Incomplete error for sample data with known expected size
- **WHEN** a sample declares a body length of 64 bytes but only 10 bytes remain
- **THEN** the error SHALL be `SflowError::Incomplete { available: 10, expected: Some(64), context: ParseContext::SampleData }`

#### Scenario: Display format for Incomplete without expected
- **WHEN** `SflowError::Incomplete { available: 3, expected: None, context: ParseContext::DatagramHeaderVersion }` is formatted
- **THEN** the output SHALL be `"Incomplete data: only 3 bytes available (datagram header version)"`

#### Scenario: Display format for Incomplete with expected
- **WHEN** `SflowError::Incomplete { available: 10, expected: Some(64), context: ParseContext::SampleData }` is formatted
- **THEN** the output SHALL be `"Incomplete data: only 10 bytes available, expected 64 (sample data)"`

### Requirement: ParseError variant uses ParseContext and ParseErrorKind
The `SflowError::ParseError` variant SHALL have the following fields:
- `offset: usize` — byte offset from the start of the datagram
- `context: ParseContext` — the parsing phase where the error occurred
- `kind: ParseErrorKind` — the category of parse error

#### Scenario: ParseError for invalid address type
- **WHEN** an unrecognized address type is encountered at offset 4
- **THEN** the error SHALL be `SflowError::ParseError { offset: 4, context: ParseContext::AgentAddress, kind: ParseErrorKind::InvalidAddressType }`

#### Scenario: ParseError wrapping a nom error
- **WHEN** parsing a flow sample fails with a nom `Eof` error
- **THEN** the error SHALL be `SflowError::ParseError { offset: 0, context: ParseContext::FlowSample, kind: ParseErrorKind::NomError(ErrorKind::Eof) }`

#### Scenario: Display format for ParseError
- **WHEN** `SflowError::ParseError { offset: 4, context: ParseContext::AgentAddress, kind: ParseErrorKind::InvalidAddressType }` is formatted
- **THEN** the output SHALL be `"Parse error at offset 4: InvalidAddressType (agent address)"`

### Requirement: SflowError retains all existing trait implementations
`SflowError` SHALL continue to implement `Debug`, `Clone`, `PartialEq`, `Eq`, `Display`, `std::error::Error`, `Serialize`, and `Deserialize`.

#### Scenario: Error trait implementation
- **WHEN** an `SflowError` is used as a `dyn std::error::Error`
- **THEN** it SHALL compile and function correctly

#### Scenario: Serde round-trip
- **WHEN** an `SflowError::ParseError` is serialized to JSON and deserialized back
- **THEN** the result SHALL equal the original value
Loading
Loading