Strongly typed error handling for remote server SSH installation flow#10429
Strongly typed error handling for remote server SSH installation flow#10429alokedesai merged 10 commits intomasterfrom
Conversation
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR introduces typed transport and SSH command errors for the remote-server SSH setup flow and moves banner rendering to structured user-facing error copy.
Concerns
- The binary check now collapses every non-success SSH command exit into
Ok(false), so abnormal failures are treated as a missing binary instead of setup failures. - User-facing stderr truncation slices a UTF-8
Stringat a fixed byte offset, which can panic when stderr contains multibyte characters.
Verdict
Found: 0 critical, 2 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
- check_binary now distinguishes test -x exit codes (0=found, 1=missing) from transport failures (255, signals) instead of collapsing all non-success into Ok(false) - Use str::floor_char_boundary for stderr truncation to avoid panicking on multibyte UTF-8 characters Co-Authored-By: Oz <oz-agent@warp.dev>
moirahuang
left a comment
moirahuang
left a comment
There was a problem hiding this comment.
i know this isn't the scope of your PR but looking at this is making me realize, do we need some sort of a CTA for when it does fail? i get that often there's not a clear action but in the cases that there are, if i was the use and saw our banner i'd wonder "ok so i have this error, now what?"
|
Yup, already planning on doing that as a followup. It depends on the error type (which requires strongly typed errors) |
Drop PlatformParseError (redundant with transport::Error), adapt RemoteShell + sh invocations to work with new SshCommandError and transport::Error types, update tests to use transport::Error. Co-Authored-By: Oz <oz-agent@warp.dev>
…warpdotdev#10429) ## Description Makes a bunch of improvements to the error UX for the remote code so that: 1) We have strongly typed errors, which allows us to separate out the error message we _log_ from the error we message to the user 2) Improves how we message the error to the user so that it's easier to understand and that the core details actually goes into the big red error box (which is the actual thing anyone is going to read) https://www.loom.com/share/82f13736fc7741c1b18152b92ddd6047?from_recorder=1&focus_title=1 ## Before <img width="1241" height="494" alt="image" src="https://github.com/user-attachments/assets/f8f949a1-19c7-48c8-b205-1965bdf61d21" /> ## After <img width="1247" height="516" alt="image" src="https://github.com/user-attachments/assets/26b8a925-2283-49eb-a1c3-bd50137efb11" /> ## Agent Mode - [x] Warp Agent Mode - This PR was created via Warp's AI Agent Mode --- [Oz conversation](https://staging.warp.dev/conversation/f40d630e-6c90-4f04-92fb-bdcd8dbc73be) Co-Authored-By: Oz <oz-agent@warp.dev> --------- Co-authored-by: Oz <oz-agent@warp.dev>
2 participants
Description
Makes a bunch of improvements to the error UX for the remote code so that:
https://www.loom.com/share/82f13736fc7741c1b18152b92ddd6047?from_recorder=1&focus_title=1
Before
After
Agent Mode
Oz conversation
Co-Authored-By: Oz oz-agent@warp.dev