Explicitly check if we actually timed out

[olegbespalov]

What?

It's an attempt to address a nil pointer from #4128

Unfortunately, we don't have a script/steps to reproduce the #4128, but after analyzing the stack trace and code it seems like one of the possible scenarios what happens is that we reached the default (30s) context timeout which causing context deadline, somewhere down the createWaitForEventHandler it exits the releases the channel and as result we didn't fill the page.

So we will always check the context error to check if it was timed out and also explicitly check if we were able to fetch non-nil page (the main purpose of the method)

Why?

Fix the panic and error explicit.

Checklist

• I have performed a self-review of my code.
• I have added tests for my changes.
• I have run linter locally (make lint) and all checks pass.
• I have run tests locally (make tests) and all tests pass.
• I have commented on my code, particularly in hard-to-understand areas.

Related PR(s)/Issue(s)

Fixes #4128

[joanlopez]

Could this context be cancelled because any reason other than context.DeadlineExceeded? Perhaps, in a future iteration of this code, we might want to introduce that explicit check or use cause.

[olegbespalov]

My understanding (@ankur22, please correct me if I am wrong) even if that happens it's still an error case

[ankur22]

Correct, any error from context is regarded as an error and we should return an error back to the user to notify them of an issue. If the context is closed at this point, it will mean that the iteration has ended.

[joanlopez]

Right! What I was wondering is whether context.DeadlineExceeded should be considered a timeout error, but context.Canceled shouldn't (but as an unknown). But both errors, yeah.

[olegbespalov]

I think I messed up the force push

Nope, it's just GH UI tricked me