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$ jakeUgh. 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.
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
...
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.gitThen, I merge in Isaac's changes:
cstrom@whitefall:~/repos/faye$ git fetch npm
remote: Counting objects: 14, done.
...
cstrom@whitefall:~/repos/faye$ git merge npm/masterNice! That looks like it will fix my error and, sure enough, it does:
Auto-merging package.json
Merge made by recursive.
Jakefile | 2 +-
Manifest.txt | 2 +-
package.json | 17 +++++++++--------
3 files changed, 11 insertions(+), 10 deletions(-)
cstrom@whitefall:~/repos/faye$ jakeTo install that with npm, I move into the build directory and use
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
npm install:cstrom@whitefall:~/repos/faye/build$ npm install .With that, I fire
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
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