Skip to content

feat(events): add SdkErrorCode typed mapping for SDK error integers#54

Merged
BlindMaster24 merged 2 commits intomainfrom
devin/1776969541-sdk-error-code
Apr 24, 2026
Merged

feat(events): add SdkErrorCode typed mapping for SDK error integers#54
BlindMaster24 merged 2 commits intomainfrom
devin/1776969541-sdk-error-code

Conversation

@devin-ai-integration
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot commented Apr 23, 2026

Summary

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 pull in teamtalk_sys::ClientError directly. 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):

  • SdkErrorCode enum (#[non_exhaustive]) with one variant per CMDERR_* (1000–3999) and INTERR_* (10000–19999) from teamtalk_sys::ClientError, plus Success (0) and Unknown(i32) for forward-compat with future SDK releases.
  • From<i32> for SdkErrorCode — lossless; unknown codes become Unknown(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 for CommandFailed, ClientError, and Ffi(FfiError::SdkError) and None for non-SDK errors (Timeout, IoError, InitFailed, etc.).

Existing fields Error::CommandFailed { code: i32, .. } and Error::ClientError { code: i32, .. } keep their i32 layout, so no downstream code breaks. Field docs now point at Error::sdk_code.

File layout: src/events.rs (256 lines) is moved to src/events/mod.rs and the new typed mapping lives in a focused sibling src/events/sdk_error.rs. No public path changes — teamtalk::events::* still re-exports everything.

Tests: crates/teamtalk/tests/sdk_error_code_tests.rs adds 11 integration cases covering: every documented ClientError FFI variant round-trips via From<i32> + as_i32; unknown positive and negative codes preserve the raw value; is_command_error / is_internal_error classification for named variants and for Unknown(_) in and out of range; Display contains 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 --check clean; cargo clippy --workspace --all-targets --all-features -- -D warnings clean; cargo test --workspace --all-features — all tests pass (existing + 11 new SdkErrorCode cases).

Review & Testing Checklist for Human

  • Skim the variant list in crates/teamtalk/src/events/sdk_error.rs against TEAMTALK_DLL/TeamTalk.h / teamtalk_sys::ClientError to make sure no CMDERR_* or INTERR_* was missed. The test from_i32_covers_every_documented_client_error_variant iterates all 44 constants currently in the bindings, so anything missing would already fail CI.
  • Confirm the choice to keep Error::CommandFailed { code: i32, .. } as i32 (with the typed sdk_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 the i32 field with SdkErrorCode directly if desired.
  • Confirm the name of AudioPreprocessorInitFailed for INTERR_SPEEXDSP_INIT_FAILED (10003). The bindings expose it under both aliases (INTERR_AUDIOPREPROCESSOR_INIT_FAILED is documented as an alias for INTERR_SPEEXDSP_INIT_FAILED with the same value). The Rust-side name leans on the newer alias for forward compatibility.

Notes

Third P1 API improvement after the ExponentialBackoff jitter PR (#51) and the StreamTypes newtype PR (#52). Branches from clean main, independent of the other open PRs, so can be merged in any order.

Next up in the API-improvement queue: TimeoutKind enum for Error::Timeout, SecretString + zeroize for passwords in LoginParams, indexed EventBus::dispatch / Router::dispatch via HashMap, then a coverage-gap test fill-in driven by scripts/audit_teamtalk_coverage.py.

The pre-existing semver CI gate is expected to remain red; release-plz handles the eventual version bump.

Link to Devin session: https://app.devin.ai/sessions/71fdd6196cb74723a2e277bb81993a9c
Requested by: @BlindMaster24


Open in Devin Review

`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-integration
Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Copy link
Copy Markdown
Contributor Author

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

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.

Open 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.
@BlindMaster24 BlindMaster24 merged commit c1ecfc1 into main Apr 24, 2026
11 of 12 checks passed
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