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

Better error handling of terminated channels #5876

Merged
merged 5 commits into from
Jun 8, 2021
Merged

Conversation

TobiGr
Copy link
Member

@TobiGr TobiGr commented Mar 22, 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

  • give a reason, why a feed could not be loaded when possible
  • allow to unsubscribe from the dedicated channel
  • show that an account has been terminated and (if provided) the reason for the termination in ChannelFragment

ToDo

  • display the information in the channel fragment
  • loading state is not changed correctly for some reason.

Fixes the following issue(s)

Relies on the following changes

Screenshots

Screenshots

APK testing

On the website the APK can be found by going to the "Checks" tab below the title and then on "artifacts" on the right.

I manipulated the subscription export to contain only URls to termianted or not available channels. Extract the json and import it for testing:
newpipe_termianted.json.zip

Due diligence

@TobiGr TobiGr added bug Issue is related to a bug youtube Service, https://www.youtube.com/ labels Mar 22, 2021
@sonarcloud
Copy link

sonarcloud bot commented Mar 22, 2021

@opusforlife2
Copy link
Collaborator

Sorely needed!

But... how do you test? xD

Copy link
Contributor

@triallax triallax left a comment

Choose a reason for hiding this comment

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

Two minor things. I guess the files should probably be formatted too.

@TobiGr TobiGr changed the title Better error handling of terminated channels when loading feed Better error handling of terminated channels Mar 31, 2021
@TobiGr
Copy link
Member Author

TobiGr commented Mar 31, 2021

But... how do you test? xD

@opusforlife2 I added a manipulated subscription export to the PR description

@Stypox Can you help me with the feed loading UI? I am unable to solve that at the moment. Just install the APK, import the subscriptions, refresh the feed and hit unsubscribe or cancel on error to see the problem.

Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

I am not sure I understand what I should test. When importing the subscriptions you provided nothing happens (i.e. no subscriptions are added). I tried both importing the json from the subscription fragment and importing the zip from content settings

app/src/main/res/values/strings.xml Show resolved Hide resolved
@TobiGr
Copy link
Member Author

TobiGr commented Apr 1, 2021

@Stypox Did you extract the json from the ZIP file before importing it via the subscription import dialog?

Anyway, here's a screen cast of the problem. As you can see, the feed keeps loading forever, although loading the items is completed.

device-2021-04-01-152332.mp4

@Stypox
Copy link
Member

Stypox commented Apr 1, 2021

@TobiGr I tried to understand how the feed loading works but I have failed so far, will try again tomorrow.

@Stypox
Copy link
Member

Stypox commented Apr 2, 2021

@TobiGr @opusforlife2 what do you think of this implementation? It now shows correctly the loaded feed and shows a dialog for each invalid subscription. If something unrelated to channel being terminated goes wrong (e.g. a network error), the bar at the top correctly shows the number of channels that could not be loaded, instead of showing "Network error". Here is a settings export (--> import it from settings): NewPipeData-20210402_120407.zip

@TobiGr
Copy link
Member Author

TobiGr commented Apr 2, 2021

Thank you. Looks good!

I fixed two small bugs (see commit message)

@TobiGr TobiGr marked this pull request as ready for review April 2, 2021 19:50
@TobiGr
Copy link
Member Author

TobiGr commented Apr 2, 2021

NewPipeData-20210402_120407.zip
The Pokemon channel is terminated now.

@opusforlife2
Copy link
Collaborator

Sorry, this slipped my mind yesterday. From the Subscriptions tab, I can see the 2 kinds of errors you pointed out: content unavailable, and channel terminated.

Refreshing a feed with the terminated Pokemon channel shows "channel terminated, wanna unsub?" so it's fine.

Doing the same with the content unavailable channel shows "got error: channel does not exist." No idea if the message is correct here. "Content unavailable" and "channel does not exist" seem like two different things.

Now the bugs:

  • If you put both the terminated and content unavailable channels in a channel group and refresh the feed, or try refreshing the What's New feed, you get one of the two dialogs, randomly selected. If you unsubscribe from one, you are shown the other as well. But if you tap cancel or tap outside the dialog box to dismiss it, you are not shown the second exception. It should show both each time.
  • Once you get either exception, cancel and go back to the Subscriptions tab. You will encounter the last shown exception as you back out of the channel group. Now keep opening one of the two valid channels and tapping Back. You keep getting the last stored exception every time you return to the Subscriptions tab upon tapping Back.
  • But if you do the same thing after unsubscribing from at least one or both channels, you don't get an exception with the same steps.

@TobiGr
Copy link
Member Author

TobiGr commented Jun 8, 2021

Rebased

Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

LGTM, thank you :-)

@TobiGr TobiGr merged commit f63a4ee into dev Jun 8, 2021
@TobiGr TobiGr deleted the terminated-channels branch June 8, 2021 08:57
@opusforlife2
Copy link
Collaborator

Did you fix the bugs I found?

This was referenced Jun 26, 2021
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 youtube Service, https://www.youtube.com/
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feed fetching errors has bad UI/UX Deleted Channels Throw An Error When Refreshing Subscription Feed
4 participants