`Path doesn't exist` when trying to open a patchset

[nanaharris]

Description:

For when trying to see details of a given patchset, the program crashes saying:

Error: 
   0: /home/ojarrisonn_/.cache/patch_hub/patchsets/accel-config.20220607025834.1740551-1-li4.zhang@intel.com.mbx: Path doesn't exist

Location:
   src/app.rs:151

How to reproduce:

1. Open accel-config list
2. Open 025. V01 | #14 | [Accel-config] Add Zcompress32, Zdecompress32, Compress and Decompres | Li Zhang
3. It crashes

Expected behavior:

The details for the given patchset should be shown

Screenshots

[INFO] [2024-10-25 18:53:57] patch-hub started
[ERROR] [2024-10-25 18:56:02] Error executing "lore_session::split_patchset(&patchset_path)": /home/ojarrisonn_/.cache/patch_hub/patchsets/accel-config.20220607025834.1740551-1-li4.zhang@intel.com.mbx: Path doesn't exist

Setup:

• Project branch: unstable
• Project commit hash: 9f14c2d
• rustc version: rustc 1.82.0 (f6e511eec 2024-10-15)
• cargo version: cargo 1.82.0 (8f40fc59f 2024-08-21)

[davidbtadokoro]

Thanks for cataloguing this 😃

The reasoning behind this bug is that patch-hub assumes (erroneously) that b4 will always succeed in finding the patchset w/ the specified version. From my experience with b4, this can fail for several reasons, but given that we are querying for an existent patch series with a valid message-ID, I see two possibilities:

1. b4 fails to find the specific version we've asked for
2. b4 fails to find the patchset itself

In the first case, we can fall back to querying without an explicit version, while the second case leaves us hanging. For now, we could follow @lorenzoberts suggestion of checking if the path points to an existent file before trying to read it.

Obs.: Ideally, we should in-house the functionality provided by b4, although I think this would escape the scope of patch-hub.

[KiranSatyaRaj]

Experienced the same while trying to see the patch-

Error: 
   0: /home/regulusnemea/.cache/patch_hub/patchsets/accel-config.20221005202317.2113-1-ramesh.thomas@intel.com.mbx: Path doesn't exist

[davidbtadokoro]

UPDATE

Reading the issue description, I've noticed it lacked a more detailed explanation of what is happening. In reality, we know exactly the point where it happens, how we can handle the issue, and what remains is the work to be done 🙃

How to reproduce it

Even though OJarrison described a valid way to reproduce the error, the patchset in question will probably be buried in the past and hard to come by. In this sense, I recommend trying to reproduce this by:

1. Consulting a mailing list in the "Latest Patchsets from" screen;
2. By trial and error (or fail and succeed 😆), find a patchset that crashes the app and returns a Path doesn't exist error;
3. Take note of this patchset as it will be useful in validating the fix.

Where the problem occurs

The root of the issue is in the function download_patchset, which is solely called in this line during the init_details_action function, where we initialize a patchset to see its details and run actions. At a high level, in the screen "Latest Patchsets from ", when the user selects a specific patchset (by hitting ENTER on an entry), init_details_action is called, and (among other things) we download the .mbox file that represents the patchset with the download_patchset function.

The download_patchset function uses b4 to download the patchset, and it sometimes fails even for a valid message-ID. However, b4 will still return status code 0 when this happens, and so this error goes undetected. Because of this, init_details_actions assumes that the patchset_path variable points to a valid file, continues, and we hit a panic in split_patchset when it tries to load the file pointed to by the variable.

My take on how we can handle this

Obs.: This is just an idea, so feel free to come up with another approach.

1. Either on init_details_actions or download_patchset, detect that b4 failed by simply checking if the path points to an existing file;
2. No matter the approach on 1, after detection, make init_details_actions return an error (before calling split_patchset);
3. This error should be handled by the callers (here and here; duplicated code, I know 😿 ) to produce a visual notification to the user that the operation has failed, without crashing the app, of course. I think creating a InfoPopUp with just a notification (an example is here) will do.