Good Thing I Wrote That Test

‹prev | My Chain | next›

Yesterday, I isolated a problem with the comet code in my (fab) game to the furthermost upstream fab.js app in:

  ( /^\/comet_view/ )
    ( broadcast_new )
    ( store_player )
    ( init_comet )
    ( player_from_querystring )

The player_from_querystring (fab) app was doing a very typical thing with the listener passed in from upstream:

      var app = out({ body: {id: q.player, x: q.x || 0, y: q.y || 0, uniq_id: uniq_id} });
      if ( app ) app();

The out local variable is a reference to the downstream listener. When I invoke the downstream listener by calling the out() function, I send a Javascript literal object representing a player in my game back downstream to be processed as the middleware deems best.

The downstream listener then does a very fab.js thing and returns a reference to itself, which gets assigned to the app local variable. The out and app local variables both refer to the same downstream listener—more on that in a bit (actually tomorrow). The purpose of the app assignment is to provide a mechanism for the upstream app to send more data if it needs to. As seen above, I did not have anymore data to send so, being a good fab.js citizen, I called the downstream listener, if downstream was even listening, with no arguments.

Empty arguments in fab.js app calls signal that the app is done. It has no reason to listen anymore and, as is the case here, has nothing else to say. This was the source of my comet bug yesterday—the empty call propagated all the way back downstream to the browser which closed the connection.

Yesterday's solution was to remove the app() call:

      // Don't care if downstream returns a listener, we send one
      // response and are done
      out({ body: {id: q.player, x: q.x || 0, y: q.y || 0, uniq_id: uniq_id} });

That solved my problem and did not break any test because I had not tested that behavior.

Now, I think that was likely a mistake. By removing the app() call, player_from_querystring is no longer being a good fab.js citizen. I doubt that I will re-use this app elsewhere, but why preclude that possibility? More important, why couple the comet-like, no-close-the-HTTP-connection behavior of the downstream apps with the implementation of player_from_querystring?

My first step in addressing this is init_comet. When player_from_querystring (or any other upstream app) says that it is done listening/sending, then init_comet should not pass the connection termination back downstream. I will do this properly, so I will first write a test to that effect. But wait, what is this? I already have a test for an empty response. What is more, it is not doing what I want:

    'without a player': {
      topic: when_upstream_replies_with(),

      'terminates the downstream connection': function(chunks) {
        assert.isUndefined(chunks[0]);
      }
    },

Aha!

It is a good thing I wrote that test. Now I know where my trouble has been all along—I am overloading the empty response in player_from_querystring. It means both that app is done processing and that it was unable to build a querystring from the URL. The former is the proper usage, I will work back down into player_from_querystring later to return a 4xx HTTP error. For now, I will simply test that init_comet sends zero chunks back downstream when the upstream player_from_querystring replies with an empty response:

    'downstream is done sending/listening': {
      topic: when_upstream_replies_with(),

      'terminates the downstream connection': function(chunks) {
        assert.equal(chunks.length, 0);
      }
    },

That fails:

cstrom@whitefall:~/repos/my_fab_game$ vows --spec test/init_comet_test.js 

♢ init_comet

  with a player
    ✓ sets a session cookie
    ✓ sends the opening HTML doc
    ✓ send 1000+ bytes to get Chrome's attention
    ✓ sends the player
  downstream is done sending/listening
    ✗ terminates the downstream connection
      » expected 0,
        got      1 (==) // init_comet_test.js:65
  with an invalid player object
    ✓ terminates the downstream connection
 
✗ Broken » 5 honored ∙ 1 broken (0.029s)

I have to write another macro function to help with the terminated downstream app, but I wind up with two topics describing the behavior that I am after:

    'downstream is done sending/listening': {
      topic: when_upstream_terminates(),

      'terminates the downstream connection': function(chunks) {
        assert.equal(chunks.length, 0);
      }
    },

    'with an invalid player object': {
      topic: when_upstream_replies_with({status:404, body:"foo"}),

      'passes thru the error': function(chunks) {
        assert.equal(chunks[0].status, 404);
      }
    }

The first topic/test asserts that an empty upstream response will result in nothing (no HTTP chunks) from the downstream perspective. The second test asserts that HTTP errors will make their way back downstream.

I get those two tests passing with a simple conditional:

function init_comet (app) {
  return function () {
    var out = this;

    return app.call( function listener(obj) {
      if (obj && obj.body && obj.body.uniq_id) {
        // do comet-y stuff here
      }
      else if (obj && obj.status) {
        out(obj)();
      }

      return listener;
    });
  };
}

And indeed, they are passing:

cstrom@whitefall:~/repos/my_fab_game$ vows --spec test/init_comet_test.js 

♢ init_comet

  with a player
    ✓ sets a session cookie
    ✓ sends the opening HTML doc
    ✓ send 1000+ bytes to get Chrome's attention
    ✓ sends the player
  downstream is done sending/listening
    ✓ terminates the downstream connection
  with an invalid player object
    ✓ passes thru the error
 
✓ OK » 6 honored (0.016s)

That is a good stopping point for now. I will move back into player_from_querystring and get proper HTTP errors working tomorrow.

Day #162