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

RESOLVED FIXED in B2G C4 (2jan on)

Status

RESOLVED FIXED
4 years ago
a year ago

People

(Reporter: aus, Assigned: aus)

Tracking

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

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [systemsfe])

Attachments

(2 attachments)

(Assignee)

Description

4 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

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

Comment 2

4 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

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

Updated

4 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

4 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

4 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

4 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

4 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

4 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

4 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: 4 years ago
Resolution: --- → FIXED

Updated

a year ago
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.