Cannot call method 'on' of undefined, in other library when using Docpad #441

Closed
dhbaird opened this Issue Mar 1, 2013 · 6 comments

Comments

Projects
None yet
2 participants

dhbaird commented Mar 1, 2013

./node_modules/.bin/docpad --version
6.21.10

node --version
v0.8.18

npm --version
1.2.2

pkgcloud version: 0.6.7-1

uname -a
Linux ubuntu-lxc 3.8.0 #6 SMP Sat Feb 23 19:11:54 MST 2013 x86_64 x86_64 x86_64 GNU/Linux

I've been having some trouble using docpad in conjunction with another library, pkgcloud. Here's what I do:

//var docpad = require('docpad'); // <--- uncomment this line to cause error in pkgcloud
var pkgcloud = require('pkgcloud');
console.log('createClient()');
var rackspace = pkgcloud.storage.createClient({
  provider: 'rackspace',
  username: 'nodejitsu',
  apiKey: 'foobar'
});
console.log('getContainers()');
rackspace.getContainers(function(err, containers) {
    console.log('Great!')
});

Actual output (when docpad is included):

createClient()
getContainers()
Great!

/home/me/node_modules/pkgcloud/lib/pkgcloud/core/base/client.js:229
      response.on('end', function () {
               ^
TypeError: Cannot call method 'on' of undefined
    at Client.request (/home/me/node_modules/pkgcloud/lib/pkgcloud/core/base/client.js:229:16)
    at Request.Client.auth [as _callback] (/home/me/node_modules/pkgcloud/lib/pkgcloud/rackspace/client.js:80:5)
    at Request.init.self.callback (/home/me/node_modules/pkgcloud/node_modules/request/main.js:127:22)
    at Request.EventEmitter.emit (events.js:99:17)
    at Request.<anonymous> (/home/me/node_modules/pkgcloud/node_modules/request/main.js:767:16)
    at Request.EventEmitter.emit (events.js:126:20)
    at IncomingMessage.Request.start.self.req.self.httpModule.request.buffer (/home/me/node_modules/pkgcloud/node_modules/request/main.js:729:14)
    at IncomingMessage.EventEmitter.emit (events.js:126:20)
    at IncomingMessage._emitEnd (http.js:366:10)
    at HTTPParser.parserOnMessageComplete [as onMessageComplete] (http.js:149:23)

Expected output (when docpad is disabled):

createClient()
getContainers()
Great!

dhbaird commented Mar 1, 2013

Here's a slight variation, that shows another problem as well,

var docpad = require('docpad'); // <--- uncomment this line to cause error in pkgcloud
var pkgcloud = require('pkgcloud');
console.log('createClient()');
var rackspace = pkgcloud.storage.createClient({
  provider: 'rackspace',
  username: 'nodejitsu',
  apiKey: 'foobar'
});
console.log('getContainers()');
rackspace.getContainers(function(err, containers) {
    console.log('here1')
    console.log(err); // <--- err won't print if docpad is enabled :(
    console.log('here2')
    console.log(containers);
    console.log('Great!')
});

Here's what actually gets printed:

createClient()
getContainers()
here1
[TypeError: Object [object Object] has no method 'slice']     <---- Problem right here!
here2
undefined
Great! 

/home/dbaird/local/src/yourpost/node_modules/pkgcloud/lib/pkgcloud/core/base/client.js:229
      response.on('end', function () {
               ^
TypeError: Cannot call method 'on' of undefined
    at Client.request (/home/dbaird/local/src/yourpost/node_modules/pkgcloud/lib/pkgcloud/core/base/client.js:229:16)
    at Request.Client.auth [as _callback] (/home/dbaird/local/src/yourpost/node_modules/pkgcloud/lib/pkgcloud/rackspace/client.js:80:5)
    at Request.init.self.callback (/home/dbaird/local/src/yourpost/node_modules/pkgcloud/node_modules/request/main.js:127:22)
    at Request.EventEmitter.emit (events.js:99:17)
    at Request.<anonymous> (/home/dbaird/local/src/yourpost/node_modules/pkgcloud/node_modules/request/main.js:767:16)
    at Request.EventEmitter.emit (events.js:126:20)
    at IncomingMessage.Request.start.self.req.self.httpModule.request.buffer (/home/dbaird/local/src/yourpost/node_modules/pkgcloud/node_modules/request/main.js:729:14)
    at IncomingMessage.EventEmitter.emit (events.js:126:20)
    at IncomingMessage._emitEnd (http.js:366:10)
    at HTTPParser.parserOnMessageComplete [as onMessageComplete] (http.js:149:23)

Here's what is expected:

createClient()
getContainers()
here1
[Error: Invalid URI "?format=json"]        <----- Expected this
here2
undefined
Great!

dhbaird commented Mar 1, 2013

I've narrowed down the source that causes the trouble:

diff --git a/src/lib/prototypes.coffee b/src/lib/prototypes.coffee
index 9654b77..34e8ab2 100755
--- a/src/lib/prototypes.coffee
+++ b/src/lib/prototypes.coffee
@@ -1,12 +1,12 @@
 String::explode or= ->
        @.split(/[,\s]+/g)

-Array::remove or= (from, to) ->
-       rest = @slice((to or from) + 1 or @length)
-       @length = (if from < 0 then @length + from else from)
-       @push.apply(this,rest)
+#Array::remove or= (from, to) ->
+#      rest = @slice((to or from) + 1 or @length)
+#      @length = (if from < 0 then @length + from else from)
+#      @push.apply(this,rest)

 Date::toShortDateString or= ->
        return @toDateString().replace(/^[^\s]+\s/,'')

dhbaird commented Mar 2, 2013

Okay, here is the culprit in the snippit below, where the Array::remove prototype is clogging things up in Pkgcloud. The "for (var i in self.before)" loop triggers an exception because it hits the Array::remove method. I found this by using the node-inspector debugger. I'm kind of stumped what the best way is to resolve this issue... Maybe to insert a type check into sendRequest to make sure the type is not an array, prior to scanning through the keys?

pkgcloud/lib/pkgcloud/core/base/client.js

  function sendRequest () {
    //
    // Setup any specific request options before 
    // making the request
    //
    if (self.before) {
      var errors = false;
      for (var i in self.before) {
        var fn = self.before[i];
        try {
          options = fn.call(self, options) || options;  // <----- fn somehow ends up being Array::remove, causes exception
        // on errors do error handling, break.
        } catch (exc) { errs.handle(exc, errback); errors = true; break; }
      }
      if (errors) { return; }
    }

    //
    // Set the url for the request based
    // on the `path` supplied.
    //
    if (typeof options.path === 'string') {
      options.path = [options.path];
    }

    //
    // Allow override of URL on parameters, otherwise use the function
    //
    if (!options.url) {
      options.url = self.url.apply(self, options.path);
    }

    // dont delete the delete. this options path thing was messing
    // request up.
    delete options.path;

    if (!errback || !callback) {
      try {
        return request(options);
      } // if request throws still return an EE
      catch (exc1) { return errs.handle(exc1); }
    } else {
      try {
        return request(options, handleRequest);
      } catch (exc2) { return errs.handle(exc2, errback); }
    }
  }
Owner

balupton commented Mar 4, 2013

Cool, do we ever use those prototypes anymore? Happy to kill them.

dhbaird commented Mar 4, 2013

Awesome, we may fix this problem in two projects at once! It's a pincer attack :) If it helps, I am working on a little patch that catches a few instances of "remove()" specifically, but I dunno about the other prototypes.

Owner

balupton commented Mar 25, 2013

Fixed in the just released v6.28.0 :) Great work.

balupton closed this Mar 25, 2013

@balupton balupton added a commit that referenced this issue Mar 25, 2013

@balupton balupton v6.28.0. Improvement.
- v6.28.0 March 25, 2013
	- Removed native prototype extensions
		- Thanks to [David Baird](https://github.com/dhbaird) for [issue
#441](#441)
		- If you were using `toShortDateString`, then we'd recommend [this
gist](https://gist.github.com/4166882) instead
		- If you were using `toISODateString`, just replace it with
`toISOString`
4717855

@balupton balupton added a commit that referenced this issue Oct 23, 2013

@balupton balupton v6.28.0. Improvement.
- v6.28.0 March 25, 2013
	- Removed native prototype extensions
		- Thanks to [David Baird](https://github.com/dhbaird) for [issue
#441](#441)
		- If you were using `toShortDateString`, then we'd recommend [this
gist](https://gist.github.com/4166882) instead
		- If you were using `toISODateString`, just replace it with
`toISOString`
70ddaae
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment