Page MenuHomePhabricator
Differential D14184

[lib] Add timeout to useWaitForConnection() hook
ClosedPublic
Actions

Authored by angelika on Tue, Jan 7, 5:43 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Jan 21, 7:13 AM
Unknown Object (File)
Sat, Jan 18, 8:59 PM
Unknown Object (File)
Sat, Jan 18, 6:09 PM
Unknown Object (File)
Sat, Jan 18, 4:33 AM
Unknown Object (File)
Sat, Jan 18, 4:11 AM
Unknown Object (File)
Fri, Jan 17, 9:58 PM
Unknown Object (File)
Fri, Jan 17, 9:18 AM
Unknown Object (File)
Fri, Jan 17, 7:50 AM
View All 30 Files
Subscribers
ashoat

Details

Reviewers
tomek
kamil
Commits
rCOMM658212f4704a: [lib] Add timeout to useWaitForConnection() hook
Summary

As per discussion here: https://phab.comm.dev/D14167 added a timeout when waiting for connection. If timeout is hit, an error is thrown.

Depends on D14167

Test Plan
  1. Have mobile and web clients that do not know each other
  2. Put a sleep() call before connecting to tunnelbroker socket (for example 20s)
  3. Close web client
  4. Send a DM on mobile to web. Make sure it's sent (checkmark)
  5. Open web client
  6. Quickly (before sleep from step 2 ends) add mobile to friends (with Settings -> friends -> add friends)

If we do it quickly enough the 5s timeout should trigger.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

angelika created this revision.Tue, Jan 7, 5:43 AM
Herald added a subscriber: ashoat. · View Herald TranscriptTue, Jan 7, 5:43 AM
angelika edited the summary of this revision. (Show Details)Tue, Jan 7, 5:43 AM
angelika edited the summary of this revision. (Show Details)
angelika added a parent revision: D14167: [lib] Wait for tunnelbroker and keyserver connection when updating relationships.
Harbormaster completed remote builds in B33197: Diff 46603.Tue, Jan 7, 6:04 AM
angelika requested review of this revision.Tue, Jan 7, 6:04 AM
ashoat added inline comments.Tue, Jan 7, 9:22 AM
lib/hooks/wait-for-connection.js
19–20 ↗(On Diff #46603)

Should these be combined?

tomek accepted this revision.Wed, Jan 8, 2:53 AM
tomek added inline comments.
lib/hooks/wait-for-connection.js
19–20 ↗(On Diff #46603)

I think so. And we should use Array type https://flow.org/en/docs/types/arrays/#toc-array-type-shorthand-syntax.

This revision is now accepted and ready to land.Wed, Jan 8, 2:53 AM
angelika updated this revision to Diff 46623.Thu, Jan 9, 2:01 AM

Review changes

Harbormaster completed remote builds in B33209: Diff 46623.Thu, Jan 9, 2:26 AM
angelika updated this revision to Diff 46632.Thu, Jan 9, 6:14 AM

Fix a bug

Harbormaster completed remote builds in B33213: Diff 46632.Thu, Jan 9, 6:32 AM
ashoat added inline comments.Thu, Jan 9, 6:48 AM
lib/hooks/wait-for-connection.js
20 ↗(On Diff #46632)

We should always prefer read-only unless we have a reason to make it writeable

angelika updated this revision to Diff 46633.Thu, Jan 9, 7:07 AM

Make properties read only

Harbormaster completed remote builds in B33214: Diff 46633.Thu, Jan 9, 7:25 AM
Closed by commit rCOMM658212f4704a: [lib] Add timeout to useWaitForConnection() hook (authored by angelika). · Explain WhyFri, Jan 10, 5:15 AM
This revision was automatically updated to reflect the committed changes.
angelika added a commit: rCOMM658212f4704a: [lib] Add timeout to useWaitForConnection() hook.

Revision Contents
Changeset List

PathSize
lib/
hooks/
29 lines

Diff 46639

View Options

lib/hooks/wait-for-connection.js

Loading...