Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Check page status code before running ledger timer #3759

Closed
da2x opened this issue Sep 6, 2016 · 2 comments
Closed

Check page status code before running ledger timer #3759

da2x opened this issue Sep 6, 2016 · 2 comments

Comments

@da2x
Copy link
Contributor

da2x commented Sep 6, 2016

Status code should be in the HTTP 2xx Success range. Anything from 3xx (redirects), 4xx (client error), or 5xx (server error) shouldn’t count against the ledger.

Should cover sites out of bandwidth, crashed sites, CloudFlare stuff, dead links, etc.

...and while we're at it, let's ensure that the page is text/html

@mrose17 mrose17 added this to the 0.12.1dev milestone Sep 6, 2016
@mrose17
Copy link
Member

mrose17 commented Sep 6, 2016

@bridiver - this looks like a one-liner! care to point me where to add that line (or add the line yourself)? thanks!

@bridiver
Copy link
Collaborator

bridiver commented Sep 6, 2016

We already cover most of these. Redirects last less than 8 seconds and 5xx errors go to about:error which are already excluded. 304 is a valid and common response that shouldn't be ignored. We can exclude 4xx pages, but it's definitely not a one-liner because we normally don't get the http status code. We either get a load success or load failure, but 4xx pages are not considered load failures by chromium. Electron has a did-get-response-details that we could possibly use in the place of the webview load, but we don't currently use it and I need to look at it more closely. With the focus/idle changes this seems like a minor issue at the moment. How long are you going to keep a 404 page focused and active?

@mrose17 mrose17 modified the milestones: 0.12.2dev, 0.12.1dev Sep 8, 2016
@bsclifton bsclifton self-assigned this Sep 13, 2016
MKuenzi pushed a commit to MKuenzi/browser-laptop that referenced this issue Sep 18, 2016
Fixes brave#3759
Auditors:
@mrose17, @bridiver

Test Plan:
1. Visit https://clifton.io
2. Stay on page for a while
3. Confirm time spent is incremented
4. Visit https://clifton.io/404
5. Stay on page for a while
6. Confirm time spent is not incremented

Udpated w/ review feedback from @bridiver
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants