Fix for argument ordering, and filling in "undefined" for missing params

[nackjicholson]

This corresponds with #64

I'd like to put this forward as the fix for that issue, but I have not run your test suite. Your tests don't really look like unit tests, so much as they are e2e tests that spin up some real servers on a few ports. This fix may very well pass them. (I'm kind of hoping putting in a PR kicks off a ci test).

If it does pass and you're satisfied with the fix, by all means merge it.

If not, close this PR. But please consider taking what I've done here and adding to it in order to resolve this issue in your own PR.

[coveralls]

Coverage increased (+0.2%) to 90.15% when pulling 817601a1f5735e70bb4ea79d342fba4035877b2d on nackjicholson:issue-64 into 507300965e25d37ee4d3379c3295d5f257b819bd on uber:master.

[coveralls]

Coverage increased (+0.2%) to 90.15% when pulling 817601a1f5735e70bb4ea79d342fba4035877b2d on nackjicholson:issue-64 into 507300965e25d37ee4d3379c3295d5f257b819bd on uber:master.

[nackjicholson]

Passed for 0.8 but not 0.10

I looked at the test that fails. It's not immediately obvious to me what that test does. At a glance it seems like it could be a timing quirk, but I don't know. I trust you probably know better than I, and I'll leave it to you. Thanks and Sorry.

[dfellis]

Hey, don't worry. Thanks for the effort you've put in so far. :)

I have several meetings today so I probably won't get to this until the afternoon, but I'll try to see why your changes break that test (some of these tests were specifically made to validate that the library behaves as expected between 0.8 and 0.10).

This also reminds me that it's time to set up 0.12 testing and confirm that it works (or not).

[dfellis]

OK: 123 assertions (73686ms)
Using reporter [text]
--------------------------|-----------|-----------|-----------|-----------|
File                      |   % Stmts |% Branches |   % Funcs |   % Lines |
--------------------------|-----------|-----------|-----------|-----------|
   lib/                   |     80.71 |     78.57 |     86.36 |     80.15 |
      client.js           |       100 |     94.12 |       100 |       100 |
      errorcode.js        |       100 |       100 |       100 |       100 |
      index.js            |       100 |       100 |       100 |       100 |
      server.js           |     71.58 |        68 |     76.92 |     71.28 |
   lib/transports/client/ |     94.76 |     83.33 |     96.67 |     95.02 |
      childProcess.js     |     83.33 |     58.33 |     85.71 |     84.44 |
      http.js             |       100 |       100 |       100 |       100 |
      tcp.js              |     97.69 |      88.3 |       100 |      97.6 |
   lib/transports/server/ |     84.06 |     68.18 |     72.41 |     87.79 |
      childProcess.js     |     31.58 |         0 |         0 |     35.29 |
      http.js             |     97.06 |     83.33 |     85.71 |       100 |
      middleware.js       |     76.92 |        50 |        60 |     83.33 |
      tcp.js              |     96.61 |     86.36 |     92.31 |     98.25 |
   lib/transports/shared/ |     91.86 |     88.57 |     83.33 |     93.75 |
      loopback.js         |       100 |       100 |        75 |       100 |
      tcp.js              |     90.14 |     87.88 |      87.5 |     92.42 |
--------------------------|-----------|-----------|-----------|-----------|
All files                 |     88.33 |     80.28 |     84.95 |     89.42 |
--------------------------|-----------|-----------|-----------|-----------|

Done
damocles@moya:~/uber/multitransport-jsonrpc(nackjicholson-issue-64)$ node --version
v0.10.31

I can't reproduce that failure at all. Going to merge and then make the code JSHint compliant.

[dfellis]

Published as 0.8.2

[nackjicholson]

@dfellis Thanks for the responsiveness sir. Glad it works!

[nackjicholson]

@dfellis Just a heads up, it could be that you can't reproduce it because you're running v0.10.31 and travis was running v0.10.36.

[dfellis]

Yeah, but Travis couldn't reproduce it, either. ;)

My guess is that there's some sort of timing issue with that test that only gets triggered on really loaded servers that I'll have to dig into and fix.