close
Skip to content

Fix crash in FilteredReadStream on task cancellation#6545

Open
emilk wants to merge 1 commit intolance-format:mainfrom
rerun-io:emilk/fix-unwrap
Open

Fix crash in FilteredReadStream on task cancellation#6545
emilk wants to merge 1 commit intolance-format:mainfrom
rerun-io:emilk/fix-unwrap

Conversation

@emilk
Copy link
Copy Markdown

@emilk emilk commented Apr 16, 2026

No description provided.

@github-actions
Copy link
Copy Markdown
Contributor

ACTION NEEDED
Lance follows the Conventional Commits specification for release automation.

The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification.

For details on the error please inspect the "PR Title Check" action.

@emilk emilk changed the title Replace .unwrap() with ? Fix crash in FilteredReadStream on task cancellation Apr 16, 2026
@emilk emilk force-pushed the emilk/fix-unwrap branch from a38ffae to 3ec544e Compare April 16, 2026 13:02
@emilk emilk marked this pull request as draft April 16, 2026 13:04
@emilk emilk force-pushed the emilk/fix-unwrap branch from 3ec544e to a38ffae Compare April 16, 2026 13:05
@emilk emilk marked this pull request as ready for review April 16, 2026 13:05
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 16, 2026

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
rust/lance/src/io/exec/filtered_read.rs 0.00% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@westonpace
Copy link
Copy Markdown
Member

This unwrap is doing panic propagation, a panic in the spawned task will propagate into the parent. This is done in quite a few places (e.g. mutexes, other spanwed tasks) throughout the code base. I can see advantages to translating those into errors but I'm curious if there is any particular reason this call site has been a bother?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants