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

Tusky not loading new toots #1022

Closed
connyduck opened this issue Feb 1, 2019 · 11 comments · Fixed by #1026
Closed

Tusky not loading new toots #1022

connyduck opened this issue Feb 1, 2019 · 11 comments · Fixed by #1026
Labels
Milestone

Comments

@connyduck
Copy link
Collaborator

Tusky is not loading new toots when pulling to refresh for one of my accounts.

the reason is it is querying the api with a wrong parameter
api/v1/timelines/home?since_id=10151730742138560z&limit=32 (note the z)
looks like a regression from #1016
will try to come up with steps to reproduce

  • Tusky Version: current master
@charlag
Copy link
Collaborator

charlag commented Feb 1, 2019

😣
Thanks for analyzing it
We should get rid of it and use min_id instead maybe but that's for another time.
I forgot that we're sending this to server in some places. Could break Pleroma too.

@connyduck
Copy link
Collaborator Author

TimelineRepository line 70 why is the id incremented here? This can't work. When sinceId ends with 0 it will then end with z -> we get error from api

@charlag
Copy link
Collaborator

charlag commented Feb 2, 2019

The increment/decrent were needed to have an overlap with existing statuses so we're sure that there's no gap. It's not needed for bottom but needed for top loading I think. We should re-think how to do this (didn't look at that line, I'm from my phone, we'll look at this today)

@connyduck
Copy link
Collaborator Author

Ok I understand, but why can't we use the old logic were we counted the number of returned statuses to determine wheter or not to show a gap? In both cases we can never be sure and can always run into the case where we show a gap and it loads nothing.

@charlag
Copy link
Collaborator

charlag commented Feb 2, 2019

Because I don't want to insert a gap into dB when it's not needed. We can do that but I cannot wrap my head around it rn.
I had a thought to use link headers. Do they point to the status from the next page or from the current?

@connyduck
Copy link
Collaborator Author

connyduck commented Feb 2, 2019

Ok lets say top status has id X, and we request at most 20 statuses from the top.
If we set since_id to X -1, and we get 20 statuses that don't contain the one with id X, we show a gap. But we could be wrong and the 21st is the one with id X. The same can happen when we count the number of returned statuses, I don't see the advantage of X - 1.

Yes link headers indicate the next and previous page.

@charlag
Copy link
Collaborator

charlag commented Feb 2, 2019

Let's say top status has ID X. We request 21 statuses with since_id to X - 1. If we see X in this set, we don't need to insert the gap, If not, then we must insert the gap.
`TimelineRepository:70

I know that the headers do indicate the next page, it was not the question. I was wondering if the header has parameter set to the item in the next list or in the current one. What I found out is that 2.6 sends prev as min_id set to the first element in the response. So it's not helping, we might as well just use min_id always but it creates some other problems.

@connyduck
Copy link
Collaborator Author

Ahhh but we always only display 20 statuses, so the gap always loads something. Ok clever. What if we don't decrement the id ourselves but use the id of the 2nd status?

@charlag
Copy link
Collaborator

charlag commented Feb 2, 2019

That's wasteful but it is the only way I've found to reliably check it.
That's the idea I had today but it's tricky because we may have situation of

top_status
gap
another status

then grabbing the second status is ambigious.
What if can use not prev but next link? It does seem to point to something outside of our array (sorry, I cannot check it, Firefox crashed 4 times and now Gedit did).

@connyduck
Copy link
Collaborator Author

That's the idea I had today but it's tricky because we may have situation of

top_status
gap
another status

then grabbing the second status is ambigious.

Ah right. But that case is very rare (how can you even get there? When all statuses between top and gap are deleted?) and we can detect it and just send a request with no since_id. It could return more than we need so its wasteful but it will work.

@charlag
Copy link
Collaborator

charlag commented Feb 2, 2019

Indeed. I can try out to hack it together today or tomorrow.
I'm sorry, I'm going through some stressful things right now and I also broke everything multiple times but I'll try to fix it. Sorry for all of that.
One upside is that now you understand the reasons behind this weird code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants