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

Delete pending missions with invalid storage #6721

Merged
merged 1 commit into from
Jul 22, 2021

Conversation

Stypox
Copy link
Member

@Stypox Stypox commented Jul 21, 2021

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

When loading pending missions from disk, sometimes an invalid mission with storage == null would be read (because of a corrupted file or with an outdated file format). This caused NullPointerExceptions down the line. So I added hasInvalidStorage() to the prelimiary checks on the mission object loaded from file, and if that's the case the related file is deleted. (each pending mission has its own file)

Fixes the following issue(s)

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.
@timjefferies @bew @castrik does this fix the problem for you?

Due diligence

@TobiGr TobiGr added bug Issue is related to a bug downloader Issue is related to the downloader labels Jul 21, 2021
Copy link
Collaborator

@XiangRongLin XiangRongLin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense and since it seems to work for the issue opener, gogo 🏃

@TobiGr TobiGr merged commit 39722a5 into TeamNewPipe:dev Jul 22, 2021
@kannes
Copy link

kannes commented Aug 3, 2021

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.

That "app" text is not clickable when not logging in to Github, here is a direct URL in the hopes that it is not login-gated: https://github.com/TeamNewPipe/NewPipe/suites/3292080699/artifacts/76861038

edit: Nope, Github serves users that are not logged in a 404 for some reason.

edit2: I extracted the zip and uploaded the apk to http://transfer.sh/19K2hb5/app-debug.apk (no warranties, don't trust a random person (me) giving you an apk, etc etc!)

edit3: Nevermind, this will install a new NewPipe app, not upgrade the existing NewPipe on the device. So it is really just useful for testing. Guess I'll have to wait for the next proper release. Cheers!

This was referenced Aug 4, 2021
@esdnm
Copy link

esdnm commented Jun 19, 2022

You can't just delete a mission if it has invalid storage, you need to handle it differently.

@opusforlife2
Copy link
Collaborator

@esdnm You're most welcome to submit a PR proposing your solution.

@Stypox Stypox deleted the pending-mission-crash branch August 4, 2022 09:48
@esdnm
Copy link

esdnm commented Sep 19, 2022

@esdnm You're most welcome to submit a PR proposing your solution.

I don't have a solution on my head. But I must say that fixing this issue in a destructive way is utterly insane.

@opusforlife2
Copy link
Collaborator

It's a matter of perspective. Another user could easily claim that leaving incomplete corrupted files lying around in storage causing bloat is what's utterly insane.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is related to a bug downloader Issue is related to the downloader
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crashing when downloading because of SAF
6 participants