Refactor Rust SDK errors to use structs with a kind() method (#1400)

* Refactor Rust SDK errors to use structs with a `kind()` method

The conventional `#[non_exhaustive] enum Error { ... }` pattern appears
safe but creates problems as a library evolves. This PR changes all
error types to the struct-with-`kind()` pattern, which also aligns with
the Azure SDK for Rust error design.

Why not a flat error enum:

- `#[non_exhaustive]` on the enum prevents exhaustive matching, but
  individual variants are still fixed. Adding a field to any variant —
  even just to improve an error message with a line number or file path
  — is a breaking change.
- Adding context data is harder than it looks. With a flat enum, new
  fields touch every affected variant and all match arms across the
  codebase. With a struct, new fields are added in one place and callers
  who don't use them are unaffected.
- A single enum conflates all failure modes, making it impossible to
  document or guarantee which variants a given function can actually
  return. Callers must handle unrelated variants they will never see, or
  accept a wildcard arm that silently swallows future additions.

The struct + kind pattern:

| Concern | Flat enum | Struct + `kind()` |
|---|---|---|
| Categorization | Match directly on variant | Call `.kind()` → `&*Kind` |
| Adding context | Breaking: add fields to variant | Non-breaking: add fields to struct |
| `non_exhaustive` | On enum; variants are fixed | Not needed on struct with only private fields |
| Simple display | Must match all variants | `format!("{err}")` — no match needed |

Callers who only want to display or propagate an error with `?` do not
need to call `.kind()` at all. Only callers who need to inspect the
failure category call `.kind()`, and they get a stable, scoped `*Kind`
enum to match against.

* Remove unnecessary error structs

Sticking with `*Kind` as a convention for error enums.

* Add backtrace support to Error struct

Enhanced the Error struct to include an optional backtrace, which is captured only when `RUST_BACKTRACE` is set. This change helps in debugging by providing context on error occurrences without inflating the Error size unnecessarily.

* Resolve PR feedback

Author: heaths

.github/copilot-instructions.md

@@ -31,6 +31,7 @@
   - Java single test: `cd java && mvn test -Dtest=CopilotClientTest` | single method: `mvn test -Dtest=ToolsTest#testToolInvocation`
   - Java format check only: `mvn spotless:check` | Build without tests: `mvn clean package -DskipTests`
   - **Java testing note:** Always use `mvn verify` without `-q` and without piping through `grep`. Never add `InternalsVisibleTo` equivalent — tests must only access public APIs.
+- Use configured LSPs for supported operations like finding references instead of pattern matching, renaming symbols, etc.
 
 ## Testing & E2E tips ⚙️
 

.github/lsp.json

@@ -21,6 +21,24 @@
         ".go": "go"
       },
       "rootUri": "go"
+    },
+    "rust-analyzer": {
+      "command": "rust-analyzer",
+      "fileExtensions": {
+        ".rs": "rust"
+      },
+      "initializationOptions": {
+        "cargo": {
+          "buildScripts": {
+            "enable": true
+          },
+          "allFeatures": true
+        },
+        "checkOnSave": true,
+        "check": {
+          "command": "clippy"
+        }
+      }
     }
   }
 }

.github/skills/rust-coding-skill/SKILL.md

@@ -13,11 +13,10 @@ Opinionated Rust rules for the Copilot Rust SDK (`rust/`). Priority order:
 
 ## Error handling
 
-The SDK's public error type is `crate::Error` (`rust/src/error.rs`). Add new
-variants there rather than introducing parallel error enums per module — every
-public failure mode is part of the API contract and should be expressible in one
-type. Internal modules can use `thiserror` enums when a richer local taxonomy
-helps; convert at the boundary.
+The SDK's public error type is `crate::Error` (`rust/src/errors.rs`). Add new
+variants to `crate::ErrorKind` rather than introducing parallel error enums
+per module — every public failure mode is part of the API contract and should
+be expressible in one type.
 
 `anyhow` is reserved for binaries and example code. Library code never returns
 `anyhow::Result` — callers can't pattern-match on `anyhow::Error`, so it would
@@ -42,7 +41,7 @@ it on shutdown. Fire-and-forget spawns silently swallow panics and outlive the
 session; don't.
 
 Blocking calls (filesystem, subprocess wait) belong in
-`tokio::task::spawn_blocking`, *not* on the async runtime. The blocking pool is
+`tokio::task::spawn_blocking`, _not_ on the async runtime. The blocking pool is
 bounded, so for genuinely long-lived workers (think: file watchers that run for
 the lifetime of a session) prefer `std::thread::spawn` with a channel back into
 async land.
@@ -81,12 +80,12 @@ Trivial field re-shaping is best inlined. Closures should stay short (under ~10
 
 **Channels, not callback closures, for event flow.** Closures fight `Send + Sync + 'static` and don't compose with `select!`. Channel choice by semantics:
 
-| Use case | Primitive |
-|---|---|
-| One producer → one consumer with backpressure | `tokio::sync::mpsc` (cap 1) or `tokio::sync::oneshot` for single value |
-| Many producers → one consumer | `tokio::sync::mpsc` |
-| One producer → many consumers, every event delivered (pub/sub) | `tokio::sync::broadcast` |
-| One producer → many consumers, only the latest value matters | `tokio::sync::watch` |
+| Use case                                                       | Primitive                                                              |
+| -------------------------------------------------------------- | ---------------------------------------------------------------------- |
+| One producer → one consumer with backpressure                  | `tokio::sync::mpsc` (cap 1) or `tokio::sync::oneshot` for single value |
+| Many producers → one consumer                                  | `tokio::sync::mpsc`                                                    |
+| One producer → many consumers, every event delivered (pub/sub) | `tokio::sync::broadcast`                                               |
+| One producer → many consumers, only the latest value matters   | `tokio::sync::watch`                                                   |
 
 For the **public** API, prefer returning `impl Stream<Item = Event>` (wrap a `broadcast::Receiver` in `tokio_stream::wrappers::BroadcastStream`). `Stream` composes with `select!`, `take`, `map`, `filter`, `timeout`. See `EventSubscription` and `LifecycleSubscription`.
 
@@ -115,7 +114,7 @@ JSON: `#[serde(rename_all = "camelCase")]` at the type level, per-field `#[serde
 Banned via `clippy.toml`. Use manual spans with `error_span!`:
 
 - **Almost always use `error_span!`**, not `info_span!`. Span level controls
-  the *minimum* filter at which the span appears. An `info_span` disappears when
+  the _minimum_ filter at which the span appears. An `info_span` disappears when
   the filter is `warn` or `error` — taking all child events with it, even
   errors. `error_span!` ensures the span is always present.
 - **Spawned tasks lose parent context.** Attach a span with `.instrument()` or
@@ -239,9 +238,9 @@ Match those exact commands locally before pushing.
 
 JSON-RPC and session-event types are generated from the Copilot CLI schema:
 
-| Source | Output |
-|---|---|
-| `nodejs/node_modules/@github/copilot/schemas/api.schema.json` | `rust/src/generated/api_types.rs` |
+| Source                                                                   | Output                                 |
+| ------------------------------------------------------------------------ | -------------------------------------- |
+| `nodejs/node_modules/@github/copilot/schemas/api.schema.json`            | `rust/src/generated/api_types.rs`      |
 | `nodejs/node_modules/@github/copilot/schemas/session-events.schema.json` | `rust/src/generated/session_events.rs` |
 
 Regenerate with:

.vscode/settings.json

@@ -14,6 +14,11 @@
   "python.testing.pytestEnabled": true,
   "python.testing.unittestEnabled": false,
   "python.testing.pytestArgs": ["python"],
+  "rust-analyzer.cargo.features": "all",
+  "rust-analyzer.check.command": "clippy",
+  "[rust]": {
+    "editor.defaultFormatter": "rust-lang.rust-analyzer"
+  },
   "[python]": {
     "editor.defaultFormatter": "charliermarsh.ruff"
   },

rust/Cargo.lock

@@ -43,7 +43,6 @@ async-trait = "0.1"
 schemars = { version = "1", optional = true }
 serde = { version = "1", features = ["derive"] }
 serde_json = "1"
-thiserror = "2"
 tokio = { version = "1", features = ["io-util", "sync", "rt", "process", "net", "time", "macros"] }
 tokio-stream = { version = "0.1", features = ["sync"] }
 tokio-util = { version = "0.7", default-features = false }
@@ -68,6 +67,18 @@ serial_test = "3"
 tempfile = "3"
 tokio = { version = "1", features = ["rt-multi-thread"] }
 
+# Integration tests that call test-support-only Client methods (e.g.
+# `from_streams_with_connection_token`, `from_streams_with_trace_provider`)
+# require the `test-support` feature because `cfg(test)` is not set on the
+# library when Cargo compiles it for integration tests.
+[[test]]
+name = "session_test"
+required-features = ["test-support"]
+
+[[test]]
+name = "protocol_version_test"
+required-features = ["test-support"]
+
 [build-dependencies]
 dirs = "5"
 flate2 = "1"

rust/Cargo.toml

@@ -9,8 +9,9 @@ use github_copilot_sdk::generated::api_types::{
 use github_copilot_sdk::generated::session_events::{
     AssistantMessageData, ExternalToolRequestedData, PermissionRequestedData, SessionEventType,
 };
+use github_copilot_sdk::subscription::RecvError;
 use github_copilot_sdk::{
-    Client, ClientOptions, EventSubscription, RecvError, ResumeSessionConfig, SessionConfig,
+    Client, ClientOptions, EventSubscription, ResumeSessionConfig, SessionConfig,
 };
 use serde_json::json;
 

rust/examples/manual_tool_resume.rs

@@ -15,7 +15,7 @@ use std::sync::Arc;
 use async_trait::async_trait;
 use github_copilot_sdk::handler::ApproveAllHandler;
 use github_copilot_sdk::session_fs::{
-    DirEntry, DirEntryKind, FileInfo, FsError, SessionFsConfig, SessionFsConventions,
+    DirEntry, DirEntryKind, FileInfo, FsError, FsErrorKind, SessionFsConfig, SessionFsConventions,
     SessionFsProvider,
 };
 use github_copilot_sdk::types::{MessageOptions, SessionConfig};
@@ -46,7 +46,7 @@ impl SessionFsProvider for InMemoryProvider {
             .lock()
             .get(path)
             .cloned()
-            .ok_or_else(|| FsError::NotFound(path.to_string()))
+            .ok_or_else(|| FsError::from(FsErrorKind::NotFound(path.to_string())))
     }
 
     async fn write_file(
@@ -69,7 +69,7 @@ impl SessionFsProvider for InMemoryProvider {
         let files = self.files.lock();
         let content = files
             .get(path)
-            .ok_or_else(|| FsError::NotFound(path.to_string()))?;
+            .ok_or_else(|| FsError::from(FsErrorKind::NotFound(path.to_string())))?;
         Ok(FileInfo::new(
             true,
             false,
@@ -101,7 +101,7 @@ impl SessionFsProvider for InMemoryProvider {
 
     async fn rm(&self, path: &str, _recursive: bool, force: bool) -> Result<(), FsError> {
         if self.files.lock().remove(path).is_none() && !force {
-            return Err(FsError::NotFound(path.to_string()));
+            return Err(FsError::from(FsErrorKind::NotFound(path.to_string())));
         }
         Ok(())
     }

rust/examples/session_fs.rs

@@ -3,7 +3,6 @@
 use async_trait::async_trait;
 use serde::{Deserialize, Serialize};
 use serde_json::Value;
-use thiserror::Error;
 
 use crate::generated::api_types::CanvasAction;
 
@@ -54,16 +53,23 @@ impl CanvasDeclaration {
 }
 
 /// Structured error returned from canvas handlers.
-#[derive(Debug, Clone, Error, Serialize, Deserialize, PartialEq, Eq)]
+#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
 #[serde(rename_all = "camelCase")]
-#[error("{code}: {message}")]
 pub struct CanvasError {
     /// Machine-readable error code.
     pub code: String,
     /// Human-readable message.
     pub message: String,
 }
 
+impl std::fmt::Display for CanvasError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}: {}", self.code, self.message)
+    }
+}
+
+impl std::error::Error for CanvasError {}
+
 impl CanvasError {
     /// Construct a new error envelope with the given code and message.
     pub fn new(code: impl Into<String>, message: impl Into<String>) -> Self {