Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Panic receiving pack if fetch interrupted #1352

Closed
LuaKT opened this issue Apr 29, 2024 · 2 comments · Fixed by #1350
Closed

Panic receiving pack if fetch interrupted #1352

LuaKT opened this issue Apr 29, 2024 · 2 comments · Fixed by #1350
Labels
acknowledged an issue is accepted as shortcoming to be fixed

Comments

@LuaKT
Copy link

LuaKT commented Apr 29, 2024

Current behavior 😯

Hi, I'm seeing a very occasional panic when fetching a repository and I interrupt it part way through receiving.

thread '<unnamed>' panicked at /home/lua/.cargo/git/checkouts/gitoxide-9add020dee14babf/035ca54/gix/src/remote/connection/fetch/receive_pack.rs:290:70:
called `Result::unwrap()` on an `Err` value: Custom { kind: Other, error: "interrupted by user" }

Unwrap is called here https://github.com/Byron/gitoxide/blob/v0.35.0/gix/src/remote/connection/fetch/receive_pack.rs#L290, is there any need to unwrap here or can errors just be ignored?

Thanks!

Expected behavior 🤔

No response

Git behavior

No response

Steps to reproduce 🕹

No response

Byron added a commit that referenced this issue Apr 30, 2024

Verified

This commit was signed with the committer’s verified signature.
Byron Sebastian Thiel
…eam (#1352)
@Byron Byron added the acknowledged an issue is accepted as shortcoming to be fixed label Apr 30, 2024
@Byron Byron linked a pull request Apr 30, 2024 that will close this issue
9 tasks
Byron added a commit that referenced this issue Apr 30, 2024

Verified

This commit was signed with the committer’s verified signature.
Byron Sebastian Thiel
…eam (#1352)
@Byron
Copy link
Member

Byron commented Apr 30, 2024

Thanks for reporting! It's fixed in the linked branch, using ? was intended here. Unwraps should never be used in production code, and if panics aren't possible, then .expect("why this can't happen") should be used instead. Thus, I don't know how that .unwrap() snuck in there, sorry for that :/.

@LuaKT
Copy link
Author

LuaKT commented Apr 30, 2024

Thanks for the quick fix!

@LuaKT LuaKT closed this as completed Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
acknowledged an issue is accepted as shortcoming to be fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants