Page MenuHomePhabricator
Differential D13382

[lib] Retry Tunnelbroker connection after closing
ClosedPublic
Actions

Authored by bartek on Sep 18 2024, 6:38 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Oct 10, 3:33 PM
Unknown Object (File)
Mon, Oct 7, 10:59 PM
Unknown Object (File)
Fri, Oct 4, 6:50 AM
Unknown Object (File)
Fri, Oct 4, 5:31 AM
Unknown Object (File)
Fri, Oct 4, 2:03 AM
Unknown Object (File)
Wed, Oct 2, 2:08 AM
Unknown Object (File)
Tue, Oct 1, 5:13 AM
Unknown Object (File)
Mon, Sep 30, 10:25 AM
View All 38 Files
Subscribers
ashoat

Details

Reviewers
kamil
tomek
Commits
rCOMM6cd543112f87: [lib] Retry Tunnelbroker connection after closing
Summary

Details in ENG-9246.
Added a retry counter variable and a timeout.

  • The effect hook has dependency on this variable
  • It is increased after each retry
  • Retry has to have interval to avoid infinite effect loop. Defaulted to 5s

Previously, effect wasn't triggered if socketState.connected was already false. Change false -> false doesn't trigger the hook, so socket wasn't recreated.

Test Plan
  • On native: Turned airplane mode on/off.
  • On web: turned wifi on/off.

Observed console logs. The connection was successfully restored after network coming back.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

bartek created this revision.Sep 18 2024, 6:38 AM
bartek held this revision as a draft.
Herald added a subscriber: ashoat. · View Herald TranscriptSep 18 2024, 6:38 AM
Harbormaster failed remote builds in B31691: Diff 44301!Sep 18 2024, 6:51 AM
bartek published this revision for review.Sep 18 2024, 7:59 AM
bartek updated this revision to Diff 44315.Sep 18 2024, 8:01 AM

Fix flow

Harbormaster completed remote builds in B31701: Diff 44315.Sep 18 2024, 8:19 AM
bartek added inline comments.Sep 18 2024, 8:40 AM
lib/tunnelbroker/tunnelbroker-context.js
293–296 ↗(On Diff #44315)

Actually should be 0, we don't retry here

kamil accepted this revision.Sep 18 2024, 9:59 AM
kamil added inline comments.
lib/tunnelbroker/tunnelbroker-context.js
91 ↗(On Diff #44315)

I am thinking about less time here, e.g. for the keyserver we have export const clientKeyserverSocketReconnectDelay = 2000; but not sure

This revision is now accepted and ready to land.Sep 18 2024, 9:59 AM
bartek added inline comments.Thu, Sep 19, 2:58 AM
lib/tunnelbroker/tunnelbroker-context.js
91 ↗(On Diff #44315)

Reflecting keyserver makes sense, I'll change to 2000

bartek updated this revision to Diff 44323.Thu, Sep 19, 3:01 AM

Address comments

Harbormaster completed remote builds in B31707: Diff 44323.Thu, Sep 19, 3:18 AM
Closed by commit rCOMM6cd543112f87: [lib] Retry Tunnelbroker connection after closing (authored by bartek). · Explain WhyThu, Sep 19, 5:30 AM
This revision was automatically updated to reflect the committed changes.
bartek added a commit: rCOMM6cd543112f87: [lib] Retry Tunnelbroker connection after closing.

Revision Contents
Changeset List

PathSize
lib/
tunnelbroker/
30 lines

Diff 44301

View Options

lib/tunnelbroker/tunnelbroker-context.js

Loading...