Skip to content

Replace panicking .unwrap()/.expect() with error propagation in production code#142

Merged
strawgate merged 2 commits into
masterfrom
copilot/replace-unwrap-expect-panics
Mar 30, 2026
Merged

Replace panicking .unwrap()/.expect() with error propagation in production code#142
strawgate merged 2 commits into
masterfrom
copilot/replace-unwrap-expect-panics

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 30, 2026

Several production code paths could crash the daemon via unrecoverable panics — lock poisoning cascades, unchecked HTTP header construction, and finish_batch() schema assertions that swallow the error type entirely.

Changes

Lock poisoning (enrichment.rs)

.expect("*lock poisoned") on RwLock replaced with .unwrap_or_else(|e| e.into_inner()). A panicking thread no longer poisons K8sPathTable, CsvFileTable, and JsonLinesFileTable for all subsequent readers/writers.

HTTP header construction (diagnostics.rs, main.rs)

.unwrap() on tiny_http::Header::from_bytes() replaced with map_err(|()| io::Error::other(...))? in functions that already return Result.

Builder finish_batch() (storage_builder.rs, streaming_builder.rs)

Return type changed from RecordBatch to Result<RecordBatch, ArrowError>; .expect() replaced with ?.

Scanner scan() (scanner.rs)

Both SimdScanner::scan() and StreamingSimdScanner::scan() now return Result<RecordBatch, ArrowError>. UTF-8 validation failures return Err(ArrowError::InvalidArgumentError(...)) instead of panicking:

// Before
std::str::from_utf8(buf).expect("SimdScanner: input is not valid UTF-8");

// After
std::str::from_utf8(buf).map_err(|e| {
    ArrowError::InvalidArgumentError(format!("SimdScanner: input is not valid UTF-8: {e}"))
})?;

#[should_panic] tests converted to assert!(result.is_err()).

Pipeline error handling (pipeline.rs)

Scan stage now handles Result from scan(), returning io::Error on failure rather than unwinding.

Callsite updates

All bench, example, fuzz-target, and integration-test callers of the changed APIs updated to .expect() / .unwrap() as appropriate for non-production code.


💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.

Copilot AI changed the title [WIP] Replace .unwrap()/.expect() panics with error propagation in production code Replace panicking .unwrap()/.expect() with error propagation in production code Mar 30, 2026
Copilot AI requested a review from strawgate March 30, 2026 01:29
@strawgate strawgate marked this pull request as ready for review March 30, 2026 01:46
@strawgate strawgate merged commit 7e15b0b into master Mar 30, 2026
5 of 7 checks passed
@strawgate strawgate deleted the copilot/replace-unwrap-expect-panics branch March 30, 2026 01:46
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.

Replace .unwrap()/.expect() panics with error propagation in production code

2 participants