Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#11400 closed defect (fixed)

Status lists itself as "dead" inaccurately

Reported by: saint Owned by: dcf
Priority: Medium Milestone:
Component: Archived/Flashproxy Version:
Severity: Keywords:
Cc: griffin@… Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

So right now, rapid network changes will trigger a false [dead] signal and seemingly remain that way. However, it is not actually dead or disabled in any way, just idle. It will still become [active], and afterwards it returns to a typical [idle] state.

This happens in a no-internet environment and when behind a captive portal. (Which, given that I am frequently behind captive portals, this likely happens more than with most users).

Proposed fixes:

  • Change [dead] to [waiting], since it seems to be waiting for a real connection
  • Ensure that flash proxies with dead/waiting status occasionally re-check connection

(this is Cupcake issue #9)

Child Tickets

Attachments (2)

0001-Make-die-really-die.patch (1.4 KB) - added by dcf 4 years ago.
0002-Don-t-die-on-network-errors-rather-try-again.patch (1.6 KB) - added by dcf 4 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 4 years ago by saint

Cc: griffin@… added

Changed 4 years ago by dcf

comment:2 Changed 4 years ago by dcf

Status: newneeds_review

Here's a summary of my thinking:

  1. The code transitions to "dead" in all the places it was meant to.
  2. However, "dead" doesn't work the way it was meant to: it was supposed to turn off all future network activity, just like "disabled" does.
  3. However however, it appears that this behavior is not what Cupcake wants--it wants to continue polling after a network error.

The "dead" state was meant as a failsafe in order to prevent runaway behavior caused by potential bugs--for example a proxy repeatedly polling a facilitator that is down for some reason. This attachment makes "dead" work the way it was meant to. It only has an effect for the places where this.die is called after a network error--during argument parsing the badge can die, but it also hasn't scheduled a timer callback yet.

However, it sounds like Cupcake doesn't want the proxy to die at the first network error. Therefore this second patch removes the calls to this.die after network errors. The effect is that the proxy will soldier on after a network error (as it has always been doing, through unintentionally)--the only difference is that the badge will no longer change color.

How do these patches work for you? An alternative to the second patch is to increment a counter or something, and then actually die (up until the next daily refresh) after a threshold is passed.

comment:3 in reply to:  2 ; Changed 3 years ago by saint

Replying to dcf:

Here's a summary of my thinking:

  1. The code transitions to "dead" in all the places it was meant to.
  2. However, "dead" doesn't work the way it was meant to: it was supposed to turn off all future network activity, just like "disabled" does.
  3. However however, it appears that this behavior is not what Cupcake wants--it wants to continue polling after a network error.

How do these patches work for you? An alternative to the second patch is to increment a counter or something, and then actually die (up until the next daily refresh) after a threshold is passed.

I agree with your line of thinking - dying works well on a standard page because refreshing the page automatically restarts flashproxy.js. In Cupcake's Chrome extension, things get trickier because it's running in the background until browser restart (Linux) or *system* restart (Win/OSX). Patch 2 has tested as fine, so is being merged. This 'bug' usually appears when someone is behind a captive portal, so it letting it attempt to poll the facilitator again seems like it would be fine. And as you point out, this is Cupcake's default behavior currently, just with an error status.

I may add a counter in addition to Patch 2 just to keep the original failsafe behavior intact. Thank you for the patch =)

Last edited 3 years ago by saint (previous) (diff)

comment:4 Changed 3 years ago by saint

Resolution: fixed
Status: needs_reviewclosed

comment:5 in reply to:  3 ; Changed 3 years ago by dcf

Replying to saint:

Replying to dcf:

Here's a summary of my thinking:

  1. The code transitions to "dead" in all the places it was meant to.
  2. However, "dead" doesn't work the way it was meant to: it was supposed to turn off all future network activity, just like "disabled" does.
  3. However however, it appears that this behavior is not what Cupcake wants--it wants to continue polling after a network error.

How do these patches work for you? An alternative to the second patch is to increment a counter or something, and then actually die (up until the next daily refresh) after a threshold is passed.

I agree with your line of thinking - dying works well on a standard page because refreshing the page automatically restarts flashproxy.js. In Cupcake's Chrome extension, things get trickier because it's running in the background until browser restart (Linux) or *system* restart (Win/OSX). Patch 2 has tested as fine, so is being merged. This 'bug' usually appears when someone is behind a captive portal, so it letting it attempt to poll the facilitator again seems like it would be fine. And as you point out, this is Cupcake's default behavior currently, just with an error status.

I may add a counter in addition to Patch 2 just to keep the original failsafe behavior intact. Thank you for the patch =)

Actually I meant to apply these patches to the main flashproxy.git. Does that work for you?

comment:6 in reply to:  5 Changed 3 years ago by dcf

Replying to dcf:

Actually I meant to apply these patches to the main flashproxy.git. Does that work for you?

It's pushed and live now. 72bd5dbb 39e61b9d.

Note: See TracTickets for help on using tickets.