resuming exits/crashes after one download

[Ramblurr]

After the workaround described in #1 I was able to sign in and start downloading photos.

The first 20-30 photos downloaded as expected, but then I wanted to test resume. So I ctrl+c'ed the process and restarted it (I'm invoking with ./gphotos-cdp -dev=true -v=true). It then downloaded one photo and exited:

$ ./gphotos-cdp  -dev=true -v=true
2020/01/25 17:48:21 Session Dir: /tmp/gphotos-cdp
2020/01/25 17:48:21 pre-navigate
2020/01/25 17:48:23 post-navigate
2020/01/25 17:48:25 Event: {Type:keyDown Modifiers:Shift Timestamp:<nil> Text: UnmodifiedText: KeyIdentifier: Code:KeyD Key:D WindowsVirtualKeyCode:68 NativeVirtualKeyCode:68 AutoRepeat:false IsKeypad:false IsSystemKey:false Location:0}
2020/01/25 17:48:25 Event: {Type:keyUp Modifiers:Shift Timestamp:<nil> Text: UnmodifiedText: KeyIdentifier: Code:KeyD Key:D WindowsVirtualKeyCode:68 NativeVirtualKeyCode:68 AutoRepeat:false IsKeypad:false IsSystemKey:false Location:0}
2020/01/25 17:48:25 Marking https://photos.google.com/photo/XYZredacted as done
OK

Running gphotos-cdp subsequent times results in the same behavior: one photo downloads with the output above and then the process exits. If I add -n=100 when executing, the behavior is the same.

If I start from scratch and this time close Chrome by clicking the X in the window decoration, I notice the process does not exit, but no output is produced. Ctrl+c'ing from here and re-rerunning results in the same behavior.

[JakeWharton]

I've found it to be somewhat probabilistic. A few restarts will generally allow it to actually resume.

[mpl]

@Ramblurr sorry, that issue flew under my radar because in my e-mails your first sentence looked like you were answering on a closed issue, and I must admit I didn't read further at the time. I think I also noticed once what you are describing. I'll investigate.

[mpl]

@Ramblurr I can reproduce and I understand why this is happening, thank you.

[mpl]

FYI, this happens because navLeft "does not work anymore", in the sense that chromedp.WaitReady("body", chromedp.ByQuery) does not seem to do the job anymore. i.e. we try moving to the next image, but we have nothing that lets us wait long enough for the page to be loaded. So when we check the location again, sometimes we do it "too early", i.e. the new location hasn't been taken into account yet. Which means our loop thinks we have navigated to the end (the beginning, to be exact) of the album and that there is nothing else left to do. @daneroo that is what you had noticed already, isn't it?

[daneroo]

Yes, it is also what I am observing, and since I was testing on 3 accounts, one of which has 31k images, I was assuredly hitting this problem regularly. I have done some experiments in an alternate re-implementation to play around with the iteration step/termination criteria. We have alternatives, but some are harder to implement in an incremental way. I would love to discuss this some more..

[mpl]

Then let's discuss. :-) 1) Is there a reliable way to check whether a navigation to the left has been initiated and has terminated, so that we can keep on using the location as a sentinel? 2) If not, what should we do instead?

[daneroo]

The approach I took, was to set up a listener on the browser side to observe for targetchanged events. (That is puppeteer speak, chromedp has a similar ListenBrowser function). When I send a navigation event I expect to be asynchronously notified that the browser page has changed, (through a channel or similar). I then wait on that notification to trigger the next navigation or implement timeout/retry/skip logic. The way I implemented it progressively, was to abstract the iteration API, so that I could have a dumb timeout approach, and compare it with this more involved event-based thing. The objective is to measure the iteration rate as well as some measure of robustness (reduce the number of skips/timeouts).

It's not done or perfect, but here is an example of the output:

Running iteration:1/3 as Daniel Lauzon (daniel.lauzon@gmail.com)
listDetail terminated { extra: { count: 7044, consecutiveZeroCounts: 5, loops: 9 } }
listDetail terminated { extra: { count: 7044, consecutiveZeroCounts: 5, loops: 23 } }
listDetail terminated { extra: { count: 7044, consecutiveZeroCounts: 5, loops: 10 } }
Running iteration:1/3 as Daniel NoJunk Lauzon (daniel.nojunk@gmail.com)
listDetail terminated { extra: { count: 30400, consecutiveZeroCounts: 5, loops: 27 } }
listDetail terminated { extra: { count: 30736, consecutiveZeroCounts: 5, loops: 79 } }
listDetail terminated { extra: { count: 13601, consecutiveZeroCounts: 5, loops: 39 } }
Running iteration:1/3 as Peru Lauzon (peru.lauzon@gmail.com)
listDetail terminated { extra: { count: 713, consecutiveZeroCounts: 5, loops: 7 } }
listDetail terminated { extra: { count: 713, consecutiveZeroCounts: 5, loops: 8 } }
listDetail terminated { extra: { count: 713, consecutiveZeroCounts: 5, loops: 8 } }

Also, I can span multiple accounts, and repeats for burn-in testing..

Here is an example of pure iteration (no downloading):

And here's a different iterator (batched) which actually does downloading and pushing to perkeep. (Downloading is skipped if the file is already present in perkeep.) In this case, we are also observing for the page's response event which is triggered when a download is actually initiated, so we don't have to poll the download directory. Notice how the second pass is faster and does not actually download any images. This is still very rough around the edges, but it's a nice playground..

[mpl]

@Ramblurr @daneroo I think I have finally found a way to wait for something that is based on events and that seems to be working. And if it indeed works, I think hopefully we'll be able to adapt it for other parts of the code.

PTAL and let me know if it works for you?

https://github.com/perkeep/gphotos-cdp/pull/11

[andaag]

This fix at least makes things a lot more reliable for me 👍.

[mpl]

@andaag thank you for the feedback.

I was hoping to get some code review, but since there does not seem to be any, I'll probably just merge as is next week unless someone objects to it in the meantime.

[daneroo]

@mpl, sorry for the radio silence, things are a little fluid around here, as I am sure they are for everyone.

I checked out the fixnavleft branch and verified it works for me.

[mpl]

@daneroo no worries, thanks for the feedback.

Take care, and good luck if you're on lockdown too.