feat(events): add SdkErrorCode typed mapping for SDK error integers#54
feat(events): add SdkErrorCode typed mapping for SDK error integers#54BlindMaster24 merged 2 commits intomainfrom
Conversation
`Error::CommandFailed`, `Error::ClientError`, and the
`FfiError::SdkError` variant previously carried the SDK's
integer `nErrorNo` as a raw `i32`, so callers had to memorise
the `CMDERR_*` / `INTERR_*` numeric constants or bring in
`teamtalk_sys::ClientError` directly.
This PR adds a typed view on top of the existing numeric field
without changing any public struct layout:
* New `teamtalk::events::SdkErrorCode` enum (`#[non_exhaustive]`)
with one variant per `CMDERR_*` (1000..=3999) and `INTERR_*`
(10000..=19999) code from `teamtalk_sys::ClientError`, plus
`Success` (0) and `Unknown(i32)` for forward compatibility
with future SDK releases.
* `From<i32> for SdkErrorCode` — lossless (unknown codes become
`Unknown(code)`, never silently dropped).
* `From<teamtalk_sys::ClientError> for SdkErrorCode` — typed
bridge from the bindings enum.
* `SdkErrorCode::as_i32`, `::name`, `::Display`,
`::is_command_error`, `::is_internal_error`, `::is_known`
— inspection helpers suitable for structured logging and
metrics.
* `Error::sdk_code(&self) -> Option<SdkErrorCode>` — non-
consuming accessor that returns the typed code for
`CommandFailed`, `ClientError`, and `Ffi(FfiError::SdkError)`
variants and `None` for non-SDK errors (`Timeout`, `IoError`,
`InitFailed`, etc.).
Strictly additive: `Error::CommandFailed { code: i32, message }`
and `Error::ClientError { code: i32, message }` keep their
existing `i32` field, so no downstream code breaks. Existing doc
comments on the fields now also point at `Error::sdk_code`.
File layout: `events.rs` (256 lines) is moved to
`events/mod.rs` and the new typed mapping lives in a focused
sibling `events/sdk_error.rs`. No public path changes —
`teamtalk::events::*` still re-exports everything.
Tests: new integration file `tests/sdk_error_code_tests.rs` with
11 cases covering:
* Every documented `ClientError` FFI variant round-trips via
`From<i32>` + `as_i32` to the same numeric code.
* Unknown codes (both positive out-of-range and negative) map to
`Unknown` with the raw value preserved.
* `is_command_error` / `is_internal_error` classification for
named variants and for `Unknown(_)` in and out of the relevant
ranges.
* `Display` includes the snake_case name and the numeric code.
* `From<teamtalk_sys::ClientError>` matches `From<i32>`.
* `Error::sdk_code` returns `Some(..)` for CommandFailed,
ClientError, and Ffi(SdkError) and `None` for the other
non-SDK error variants.
* All `name()` values are unique.
Local verification:
* cargo fmt --all clean.
* cargo clippy --workspace --all-targets --all-features -- -D
warnings clean.
* cargo test --workspace --all-features — all tests pass,
including the 11 new SdkErrorCode cases.
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
There was a problem hiding this comment.
Devin Review found 1 potential issue.
⚠️ 1 issue in files not directly in the diff
⚠️ Missing SdkErrorCode re-export from lib.rs breaks crate-root access pattern (crates/teamtalk/src/lib.rs:80)
SdkErrorCode is a new public type exposed via crates/teamtalk/src/events/mod.rs:4, but it is not re-exported from crates/teamtalk/src/lib.rs:80 where every other public type from the events module (ConnectionState, Error, Event, FfiError, Result) is re-exported at the crate root. This means teamtalk::SdkErrorCode does not resolve, and users must use the longer teamtalk::events::SdkErrorCode path — inconsistent with how the rest of the events API is consumed. This violates the AGENTS.md rule: "Keep public API surface shallow: expose through lib.rs and module mod.rs."
View 3 additional findings in Devin Review.
Devin Review flagged that SdkErrorCode was reachable only via teamtalk::events::SdkErrorCode, while every other public type from the events module (ConnectionState, Error, Event, FfiError, Result) is already re-exported at teamtalk::*. This hop makes the new type feel like a second-class citizen for the common import pattern 'use teamtalk::SdkErrorCode;' and violates the 'Keep public API surface shallow' guideline from AGENTS.md. Add SdkErrorCode to the crate-root pub use so callers can write both teamtalk::SdkErrorCode and teamtalk::events::SdkErrorCode interchangeably.
Summary
Error::CommandFailed,Error::ClientError, and theFfiError::SdkErrorvariant previously carried the SDK's integernErrorNoas a rawi32, so callers had to memorise theCMDERR_*/INTERR_*numeric constants or pull inteamtalk_sys::ClientErrordirectly. This PR adds a typed view on top of the existing numeric field without changing any public struct layout (strictly additive — no downstream breakage).New API (all in
teamtalk::events):SdkErrorCodeenum (#[non_exhaustive]) with one variant perCMDERR_*(1000–3999) andINTERR_*(10000–19999) fromteamtalk_sys::ClientError, plusSuccess(0) andUnknown(i32)for forward-compat with future SDK releases.From<i32> for SdkErrorCode— lossless; unknown codes becomeUnknown(code)and are never silently dropped.From<teamtalk_sys::ClientError> for SdkErrorCode— typed bridge from the bindings enum.SdkErrorCode::as_i32/name/Display/is_command_error/is_internal_error/is_known— inspection helpers suitable for structured logging and metrics.Error::sdk_code(&self) -> Option<SdkErrorCode>— non-consuming accessor that returns the typed code forCommandFailed,ClientError, andFfi(FfiError::SdkError)andNonefor non-SDK errors (Timeout,IoError,InitFailed, etc.).Existing fields
Error::CommandFailed { code: i32, .. }andError::ClientError { code: i32, .. }keep theiri32layout, so no downstream code breaks. Field docs now point atError::sdk_code.File layout:
src/events.rs(256 lines) is moved tosrc/events/mod.rsand the new typed mapping lives in a focused siblingsrc/events/sdk_error.rs. No public path changes —teamtalk::events::*still re-exports everything.Tests:
crates/teamtalk/tests/sdk_error_code_tests.rsadds 11 integration cases covering: every documentedClientErrorFFI variant round-trips viaFrom<i32>+as_i32; unknown positive and negative codes preserve the raw value;is_command_error/is_internal_errorclassification for named variants and forUnknown(_)in and out of range;Displaycontains the snake_case name and the numeric code;From<teamtalk_sys::ClientError>matchesFrom<i32>;Error::sdk_codereturnsSome(..)forCommandFailed,ClientError, andFfi(SdkError)andNonefor the other non-SDK error variants; allname()values are unique.Local verification:
cargo fmt --all --checkclean;cargo clippy --workspace --all-targets --all-features -- -D warningsclean;cargo test --workspace --all-features— all tests pass (existing + 11 new SdkErrorCode cases).Review & Testing Checklist for Human
crates/teamtalk/src/events/sdk_error.rsagainstTEAMTALK_DLL/TeamTalk.h/teamtalk_sys::ClientErrorto make sure noCMDERR_*orINTERR_*was missed. The testfrom_i32_covers_every_documented_client_error_variantiterates all 44 constants currently in the bindings, so anything missing would already fail CI.Error::CommandFailed { code: i32, .. }asi32(with the typedsdk_code()accessor) instead of introducing a breaking change on the struct field. Rationale: keeps this PR strictly additive and reviewable in isolation; a later major-bump PR can replace thei32field withSdkErrorCodedirectly if desired.AudioPreprocessorInitFailedforINTERR_SPEEXDSP_INIT_FAILED(10003). The bindings expose it under both aliases (INTERR_AUDIOPREPROCESSOR_INIT_FAILEDis documented as an alias forINTERR_SPEEXDSP_INIT_FAILEDwith the same value). The Rust-side name leans on the newer alias for forward compatibility.Notes
Third P1 API improvement after the
ExponentialBackoffjitter PR (#51) and theStreamTypesnewtype PR (#52). Branches from cleanmain, independent of the other open PRs, so can be merged in any order.Next up in the API-improvement queue:
TimeoutKindenum forError::Timeout,SecretString+zeroizefor passwords inLoginParams, indexedEventBus::dispatch/Router::dispatchviaHashMap, then a coverage-gap test fill-in driven byscripts/audit_teamtalk_coverage.py.The pre-existing
semverCI gate is expected to remain red;release-plzhandles the eventual version bump.Link to Devin session: https://app.devin.ai/sessions/71fdd6196cb74723a2e277bb81993a9c
Requested by: @BlindMaster24