Eliminate an unnecessary HTTP request during login

[orenyomtov]

This change eliminates requesting and downloading the entire FB home page (~160kb) every login.

Another HTTP request can be eliminated when logging in using session cookies by removing or not self.isLoggedIn() from line 74 of client.py, because if the session_cookies are not valid, setSession() would surely return False, because of the many exceptions that _postLogin() will be throwing.

What do you think?

[orenyomtov]

I see this applies to logging in with session cookies.

In a regular login the check is made here:
https://github.com/carpedm20/fbchat/blob/master/fbchat/client.py#L258

# Sometimes Facebook tries to show the user a "Save Device" dialog
        if 'save-device' in r.url:
            r = self._cleanGet(self.req_url.SAVE_DEVICE)

        if 'home' in r.url:

If Facebook tries to show the user a "Save Device" dialog... doesn't that mean the log in was successful already? Is the followup request (2 requests actually, because of an HTTP redirect) necessary?

[madsmtm]

I think the reason for making sure to pass the 'save-device' dialog was mostly just a hotfix, not really thought out.

But maybe it also helped making sure that when we want to fetch the home page in _postLogin, at line 210, the result is actually the home page, and not the save-device dialog again?

Though that seems unlikely, and you're probably right in that it could be removed

Though really should find another way to test if the user successfully logged in, since Facebook keep adding a bunch of alerts and such all the time, that will break our code. For example, I've seen a "Changes to match GDPR" alert recently, and I once saw a "Happy Birthday" alert, too.

Maybe it's possible to just check if the c_user cookie is set?

[orenyomtov]

Checking for the c_user does sound like a better approach.

Regarding the concern of wanting to make sure to pass the 'save-device' dialog: because the previous request is made using allow_redirects=True (which is the default), the 'save-device' dialog will be already requested (hence the check of r.url and not r.headers['Location']). So it's being requested twice now.

[madsmtm]

Hmm, not exactly sure what you mean, are you saying that we already have in line 259, and that line 260 is unnecessary?

But maybe we should just drop this discussion, and attempt to implement the c_user approach? My biggest concern is getting 2FA to work properly, I've already tested, and the save-device popup doesn't cause any trouble. Do you have a 2FA enabled account, and want to try attempt working on it, or should I enable it on my own account and attempt it myself?

Anyway, your recent PRs are definitely without a problem, I'll merge them, but probably only release a new version when we're done

[orenyomtov]

Thanks for merging them all so quickly!

I'll be out for further developments for the time being, so feel free to tackle it yourself

[madsmtm]

This fails if the Location header isn't set, as when the client isn't logged in, and therefore isn't redirected. I went ahead and fixed it with by testing if 'Location' in r.headers

[orenyomtov]

Good catch!