[sockit-to-me] Enable setPollTimeout to influence 'connect' timeout

RESOLVED FIXED in B2G C4 (2jan on)

Status

Testing
JSMarionette
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: aus, Assigned: aus)

Tracking

unspecified
B2G C4 (2jan on)
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [systemsfe])

Attachments

(2 attachments)

(Assignee)

Description

3 years ago
Currently calling setPollTimeout has no affect on timeouts for calls during connect. We should also prevent connect from being called twice without close being called.
(Assignee)

Comment 1

3 years ago
Pull request coming soon.
Status: NEW → ASSIGNED
(Assignee)

Comment 2

3 years ago
Created attachment 8548570 [details] [review]
Pull Request - Enable setPollTimeout to influence connect timeout. Do not allow connect to be called while already connecting.
Attachment #8548570 - Flags: review?(kgrandon)
(Assignee)

Comment 3

3 years ago
Also removes support for node 0.8.x.
(Assignee)

Updated

3 years ago
Whiteboard: [systemsfe]
Comment on attachment 8548570 [details] [review]
Pull Request - Enable setPollTimeout to influence connect timeout. Do not allow connect to be called while already connecting.

I took a look through the code. I'm not super familiar with this project, or sockets, so I probably couldn't give you the best review. You may want to consider flagging someone else if you would like a better design review. For now I think I'm fine R+ stamping this due to the desire to get the issue fixed.

Please land this and bump the version in npm. Thanks!
Attachment #8548570 - Flags: review?(kgrandon) → review+
This looks good to me should help narrow down any cases that have these issues...
Hey - can one of you guys get this landed/do the janky linux build/published to npm? Thanks!
Flags: needinfo?(gaye)
Flags: needinfo?(aus)
(Assignee)

Comment 7

3 years ago
If gaye isn't set-up to build this thing anymore I can set-up a VM with the correct Ubuntu version to build this today.
Flags: needinfo?(aus)
Ok, I think :gaye volunteered when I chatted with him earlier - just didn't want you guys to duplicate efforts. As long as you guys are talking things should be fine :)
(Assignee)

Comment 9

3 years ago
Commit (master): https://github.com/mozilla-b2g/sockit-to-me/commit/ea23d94cc72d7232bf65aa94874da5bb9901c991

Version bumped to v0.2.4. Still need to build on Ubuntu 13.04 prior to releasing via NPM.

Will keep this open until we push to NPM.
(Assignee)

Comment 10

3 years ago
Actually, it's on Ubuntu Precise 12.04 (i386 and amd64). Ouch. Luckily Amazon AWS still has those. Binaries are building now.
Flags: needinfo?(gaye)
(Assignee)

Comment 11

3 years ago
Created attachment 8549128 [details] [review]
Pull Request - New binaries for Ubuntu 12.04 i386 and amd64
Attachment #8549128 - Flags: review?(gaye)
(Assignee)

Comment 12

3 years ago
Comment on attachment 8549128 [details] [review]
Pull Request - New binaries for Ubuntu 12.04 i386 and amd64

r=me, just need to keep this moving as fast as possible.
Attachment #8549128 - Flags: review?(gaye) → review+
(Assignee)

Comment 13

3 years ago
Commit (master): https://github.com/mozilla-b2g/sockit-to-me/commit/bd2b8c17dba90db325f87f9d6bbe958668ec8912

New binaries landed. New node module published to NPM. Fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.