Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

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

Comments

@dhbaird
Copy link

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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); }
    }
  }

@balupton
Copy link
Member

balupton commented Mar 4, 2013

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

@dhbaird
Copy link
Author

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.

@balupton
Copy link
Member

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

balupton added a commit that referenced this issue Mar 25, 2013
- 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`
balupton added a commit that referenced this issue Oct 23, 2013
- 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`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants