Tuesday, August 24, 2010

Fork Faye

‹prev | My Chain | next›

Up tonight, I try to add some error handling to faye channel subscriptions. Yesterday, I isolated the problem in a try-catch blocks in the addListener method:
   request.addListener('response', function(stream) {
response = stream;
Faye.withDataFor(response, function(data) {
try {
self.receive(JSON.parse(data));
} catch (e) {
retry();
}

});
});
The catch block completely ignores any exceptions preferring instead to retry() the original request. A retry sounds like a great idea when there is a problem with the response, but not when there is an error in my code. In my experience, I am far more likely to write bad code than I am to receive a bad response.

I consider throwing the error depending on the type, but opt against that. Blacklisting/whitelisting types are bound to miss something. Instead, I simply add some logging:
    request.addListener('response', function(stream) {
response = stream;
Faye.withDataFor(response, function(data) {
try {
self.receive(JSON.parse(data));
} catch (e) {
var stack = e.stack.split("\n");
self.error(stack[0] + " " + stack[1].replace(/ +/, ''));
self.debug(e.stack);


retry();
}
});
});
I always warn that something has gone wrong. The first two lines include the message itself and the line at which the exception was thrown. If I am at debug loglevel, then I can see the whole stack trace. That seems like a reasonable compromise.

Let's see if it works:
node> var faye = require("faye");
node> var client = new faye.Client("http://localhost:4011/faye");
node> var puts = require("sys").puts;
node> var subscription = client.subscribe("/foo", function(m) {puts("1"); asdf.foo.bar; puts("2");});
node> client.publish("/foo", "test 01")
node> 1
2010-08-24 19:55:13 [ERROR] [Faye.Transport] ReferenceError: asdf is not defined at [object Context]:1:69

Nice!

So far, I have been editing the faye-node.js file directly in my npm library. That is not exactly a recipe for maintainability. So I fork faye and apply the logging change.

To build the library for node, I issue the jake command (it's make for Javascript):
cstrom@whitefall:~/repos/faye$ jake
core src /build/core.js 30 kB
core min /build/core-min.js 15 kB
faye-browser src /build/faye-browser.js 56 kB
faye-browser min /build/faye-browser-min.js 23 kB
faye-node src /build/faye-node.js 57 kB
faye-node min /build/faye-node-min.js 30 kB
/home/cstrom/.rvm/rubies/ruby-1.8.7-p249/lib/ruby/1.8/fileutils.rb:1201:in `stat': No such file or directory - README.txt (Errno::ENOENT)
from /home/cstrom/.rvm/rubies/ruby-1.8.7-p249/lib/ruby/1.8/fileutils.rb:1201:in `lstat'
from /home/cstrom/.rvm/rubies/ruby-1.8.7-p249/lib/ruby/1.8/fileutils.rb:1179:in `stat'
from /home/cstrom/.rvm/rubies/ruby-1.8.7-p249/lib/ruby/1.8/fileutils.rb:1261:in `copy_file'
from /home/cstrom/.rvm/rubies/ruby-1.8.7-p249/lib/ruby/1.8/fileutils.rb:463:in `copy_file'
from /home/cstrom/.rvm/rubies/ruby-1.8.7-p249/lib/ruby/1.8/fileutils.rb:383:in `cp'
from /home/cstrom/.rvm/rubies/ruby-1.8.7-p249/lib/ruby/1.8/fileutils.rb:1396:in `fu_each_src_dest'
from /home/cstrom/.rvm/rubies/ruby-1.8.7-p249/lib/ruby/1.8/fileutils.rb:1412:in `fu_each_src_dest0'
from /home/cstrom/.rvm/rubies/ruby-1.8.7-p249/lib/ruby/1.8/fileutils.rb:1394:in `fu_each_src_dest'
from /home/cstrom/.rvm/rubies/ruby-1.8.7-p249/lib/ruby/1.8/fileutils.rb:382:in `cp'
from /home/cstrom/repos/faye/Jakefile:13
from /home/cstrom/repos/faye/Jakefile:12:in `each'
from /home/cstrom/repos/faye/Jakefile:12
...
Ugh. Before trying to solve this myself, I seem to recall that Isaac Schlueter (the npm guy) had a fork of faye on the github network graph. Sure enough, he does have a fork of faye and seems to have solved this problem.

I adapt the guide on forking and pulling upstream changes to pull in Isaac's changes instead. I add Isaac's repository as a remote repo and fetch it into my local git DB:
cstrom@whitefall:~/repos/faye$ git remote add npm git://github.com/isaacs/faye.git
cstrom@whitefall:~/repos/faye$ git fetch npm
remote: Counting objects: 14, done.
...
Then, I merge in Isaac's changes:
cstrom@whitefall:~/repos/faye$ git merge npm/master
Auto-merging package.json
Merge made by recursive.
Jakefile | 2 +-
Manifest.txt | 2 +-
package.json | 17 +++++++++--------
3 files changed, 11 insertions(+), 10 deletions(-)
Nice! That looks like it will fix my error and, sure enough, it does:
cstrom@whitefall:~/repos/faye$ jake
core src /build/core.js 30 kB
core min /build/core-min.js 15 kB
faye-browser src /build/faye-browser.js UP-TO-DATE
faye-browser min /build/faye-browser-min.js UP-TO-DATE
faye-node src /build/faye-node.js UP-TO-DATE
faye-node min /build/faye-node-min.js 30 kB
To install that with npm, I move into the build directory and use npm install:
cstrom@whitefall:~/repos/faye/build$ npm install .
npm it worked if it ends with ok
npm cli [ 'install', '.' ]
npm version 0.1.26
...
npm build Success: faye@0.5.2
npm ok
With that, I fire node-repl back up and it looks like all is well:
node> var faye = require("faye");
node> var client = new faye.Client("http://localhost:4011/faye");
node> var puts = require("sys").puts;
node> var subscription = client.subscribe("/foo", function(m) {puts("1"); asdf.foo.bar; puts("2");});
node> client.publish("/foo", "test 01")
node> 1
2010-08-24 20:59:13 [ERROR] [Faye.Transport] ReferenceError: asdf is not defined at [object Context]:1:69

Yay!

That is a fine stopping point for tonight. Tomorrow, I hope that I will not require these debugging aids, but I probably will.


Day #205

No comments:

Post a Comment