Skip to content

bug(update-list): retry thread can panic on network errors #916

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

Open
3 tasks done
Rudxain opened this issue Mar 27, 2025 · 0 comments
Open
3 tasks done

bug(update-list): retry thread can panic on network errors #916

Rudxain opened this issue Mar 27, 2025 · 0 comments
Labels
bug Something isn't working

Comments

@Rudxain
Copy link
Member

Rudxain commented Mar 27, 2025

Describe the bug

Quoted:

I was further analyzing this code, and I realized "wait, there's absolutely no reason why this should ever panic. Why not simply return the local list?"

The reason why I started analyzing it, is because I actually got a "response should be Ok type" panic recently (network error, not GH's fault). The app-list view got "soft-locked" in the "downloading latest list" state, waiting for the thread spawned by retry

Expected behavior

The local (embedded or cached) list should always be used as fall-back, to avoid needless panic

You have a solution?

Quoted:

I was already thinking of a solution. It's not as easy as "replace all expects by return"

I prompted Copilot "I want to avoid all potential panics here, and fall-back to the local list as a last-resort". This is the response:

To avoid potential panics and fall back to the local list as a last resort, we need to handle errors gracefully and ensure that any failure in the remote fetch operation does not cause a panic. Here's a modified version of the code that achieves this:

Ok(data) => {
    // TODO: max resp size is 10MB, list is ~1.3MB;
    // TODO: https://github.com/Universal-Debloater-Alliance/universal-android-debloater-next-generation/discussions/608
    let text = match data.into_string() {
        Ok(text) => text,
        Err(e) => {
            warn!("Could not convert response to string: {}", e);
            return retry::OperationResult::Err(get_local_lists());
        }
    };

    if let Err(e) = fs::write(cached_uad_lists.clone(), &text) {
        warn!("Unable to write file: {}", e);
        return retry::OperationResult::Err(get_local_lists());
    }

    let list: PackageHashMap = match serde_json::from_str(&text) {
        Ok(list) => list,
        Err(e) => {
            warn!("Unable to parse response: {}", e);
            return retry::OperationResult::Err(get_local_lists());
        }
    };

    OperationResult::Ok(list)
}

In this modification:

  • We use match statements to handle potential errors from into_string, fs::write, and serde_json::from_str.
  • If any of these operations fail, we log a warning and return the local list using get_local_lists() instead of panicking.
  • This ensures that the function will always return a valid PackageHashMap, either from the remote source or from the local cache as a fallback.

Operating System

Linux

Distro version

Debian 13

Window System + Manager

No response

Desktop Environment

No response

ADB version

No response

Provide logs

None

Acknowledgements

  • This issue is not a duplicate of an existing bug report.
  • I have chosen an appropriate title.
  • All requested information has been provided properly.
@Rudxain Rudxain added the bug Something isn't working label Mar 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant