Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement new mux layer (TCPServersManager) #358

yaronyg opened this issue Dec 1, 2015 · 2 comments

Implement new mux layer (TCPServersManager) #358

yaronyg opened this issue Dec 1, 2015 · 2 comments


Copy link

@yaronyg yaronyg commented Dec 1, 2015

Please see

Remaining work:


  • Finish terminateOutgoingConnection and connect to terminateListener in thaliMobileNativeWrapper and surface in thaliMobile


  • Figure out the maximum call stack size exceeded crashing bug, we know Winston is involved but is this a bug in Winston or some kind of weird loop on our part?
  • Review createNativeListener to find all the places where the CBs make bad assumptions
  • Punch up the createNativeListener tests to test the weak points where things can fail
  • Add test that we properly close incoming connections when they time out
  • Fix incomingConnectionState events aren't emitting correct values
  • Fix the createNativeListener tests so they will work when we are using the connected test runner
  • Check if routerPortConnectionFailed is emitted correctly & fix tests
  • Implement terminateIncomingConnection & test
  • Connect terminateIncomingConnection to terminateConnection (whose name we need to change) in thaliMobileNativeWrapper


  • Fix tests in testThaliMobileNativeWrapper that call trivialBadEndtoEndTest, there is a bug there that is sourced somewhere in this code
  • Validate that all of our requirements in jsdoc are met in the code
  • Check if failedconnection is emitted correctly & fix tests
  • Make sure in non-failure scenarios that we don't emit failedconnection, the fact that we call destroy on the mux can cause spurious errors
  • Check to see if we are using any other native methods and if so that they are expecting strings
  • Check all the cbs in the forward connection code for bad assumptions, also check the pipes for possible closed objects
  • Check forward connection weak points to make sure we clean up properly
  • Check the code that creates a listener in response to a peer availability event and make sure that at some point we clean port up.
  • Figure out if the start interface on tcpServersManager really needs to return the local port it is listening on for connections from the native interface. Does or should anyone but tcpServersManager care about that value?
  • Make sure all multiplex streams have a finish handler that calls destroy on the stream so that our clean up code is in close (which isn't guaranteed to always be called).
  • Make sure we have error handlers on all streams and other objects because otherwise we can get unhandled exceptions.
  • In general we should prefer destroy to end for closing most streams in most cases because it guarantees to kill everything regardless of its state.
  • Figure out how we can convince the peer code (e.g. outgoing connections) to behave itself when we are using the connection manager for tests.
  • Add code to test that when we pick the oldest peer to kill we pick the right one because I fixed a bug in the code that indicates we aren't testing this at all.
  • We have outgoing.on('timeout'... but no where do we actually set the timeout so it won't ever fire. We need to set the timeout and test that it works.
  • We don't wait for listen to call a callback proving it is available, isn't that an issue with resolving the promise?
@yaronyg yaronyg added 0 - Icebox and removed 0 - Icebox labels Dec 1, 2015
@yaronyg yaronyg closed this Jan 5, 2016
@yaronyg yaronyg added 4 - Done and removed 1 - Backlog labels Jan 5, 2016
@yaronyg yaronyg reopened this Jan 5, 2016
@yaronyg yaronyg closed this Jan 5, 2016
@yaronyg yaronyg reopened this Jan 5, 2016
@yaronyg yaronyg added 2 - Ready and removed 4 - Done labels Jan 5, 2016
@yaronyg yaronyg added this to the New Infra milestone Jan 11, 2016
@yaronyg yaronyg changed the title Implement new mux layer Implement new mux layer (TCPServersManager) Jan 28, 2016
@yaronyg yaronyg added 3 - Working and removed 2 - Ready labels Feb 9, 2016
@yaronyg yaronyg assigned yaronyg and unassigned tobybrad Mar 4, 2016
@yaronyg yaronyg added 2 - Ready and removed 3 - Working labels Mar 4, 2016
@yaronyg yaronyg added 2 - Ready and removed 3 - Working labels Mar 21, 2016
@yaronyg yaronyg added 1 - Backlog and removed 2 - Ready labels Apr 6, 2016
@yaronyg yaronyg removed their assignment Jul 15, 2016
@yaronyg yaronyg closed this Aug 8, 2016
@yaronyg yaronyg added 4 - Done and removed 1 - Backlog labels Aug 8, 2016
Copy link
Member Author

@yaronyg yaronyg commented Aug 8, 2016

We are getting rid of the old iOS code so we don't need this work item anymore

@yaronyg yaronyg removed the ready label Aug 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.