3rd-Eden / useragent

Useragent parser for Node.js, ported from browserscope.org
MIT License
897 stars 137 forks source link

[bug] TypeError: Cannot read property 'length' of undefined #30

Closed binarykitchen closed 11 years ago

binarykitchen commented 11 years ago

Looks like your latest release introduced a new bug:

/home/michael.heuberger/projects/binarykitchen/code/videomail/node_modules/useragent/index.js:13
  , osparserslength = osparsers.length;
                               ^
TypeError: Cannot read property 'length' of undefined
    at Object.<anonymous> (/home/michael.heuberger/projects/binarykitchen/code/videomail/node_modules/useragent/index.js:13:32)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)
    at Object.<anonymous> (/home/michael.heuberger/projects/binarykitchen/code/videomail/app/lib/util/useragent.js:3:17)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)

Process finished with exit code 8

Any change you can fix that pretty soon? Thanks!

binarykitchen commented 11 years ago

PS: Contents of ./lib/regexps are empty ... probably the update via browserscope.org failed and deleted whole contents of the file which made my app crash.

binarykitchen commented 11 years ago

I think it should be a bit different:

3rd-Eden commented 11 years ago

So you are running it with the auto updating enabled? You probably had this crash because github was having servere database issues what caused them to go offline.

Having that said, it should not delete the file when an update fails. I'll see if I can make the updater more robust.

binarykitchen commented 11 years ago

correct, i had the auto updater enabled. your !err addition in that if condition is the solution. thanks!

3rd-Eden commented 11 years ago

Published 2.0.6

nfo commented 11 years ago

I had the same error with the version 2.0.6, and I enabled the update via browerscope.org.

/var/app/current/node_modules/useragent/index.js:13
  , osparserslength = osparsers.length;
                               ^
TypeError: Cannot read property 'length' of undefined
    at Object.<anonymous> (/var/app/current/node_modules/useragent/index.js:13:32)
    at Module._compile (module.js:449:26)
    at Object.Module._extensions..js (module.js:467:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.require (module.js:362:17)
    at require (module.js:378:17)
    at Object.<anonymous> (/var/app/current/app.js:51:17)
    at Module._compile (module.js:449:26)
    at Object.Module._extensions..js (module.js:467:10)

I can't SSH the servers anymore, to see if it happened because of empty regexps (The EC2 instances are terminated, they were handled by Elastic Beanstalk).

len-am commented 10 years ago

We're still seeing this error with 2.0.7: "stack": [ "TypeError: Cannot read property 'length' of undefined", " at Object. (/opt/appcelerator/360/node_modules/useragent/index.js:13:32)", " at Module._compile (module.js:456:26)", " at Module._extensions..js (module.js:474:10)", " at Module.load (module.js:356:32)", " at Module._load (module.js:312:12)", " at Module.require (module.js:364:17)", " at require (module.js:380:17)", " at Object. (/opt/appcelerator/360/api/node_modules/data/auth.js:3:14)", " at Module._compile (module.js:456:26)", " at Module._extensions..js (module.js:474:10)" ]

3rd-Eden commented 10 years ago

@len-am The format of the browserscope file changed, so this will most likely be the issue.

len-am commented 10 years ago

@3rd-Eden I didn't get a chance to inspect the regexps.js to see if it's the same issue. I just restored my module and things are running fine now

The format of the browserscope file changed, so this will most likely be the issue.

Any way to guard against this other than to not enable auto update?

klall commented 7 years ago

We seem to run into this scenario once in a while as well when we do an npm update.

npm update will run node_modules/useragent/bin/update.js

If I trigger an error in any of the scenarios below, I generally don't have an empty file:

exports.update = function update(callback) {
  // Prepend local additions that are missing from the source
  fs.readFile(exports.before, 'utf8', function reading(err, before) {
    if (err) return callback(err);

    // Fetch the remote resource as that is frequently updated
    request(exports.remote, function downloading(err, res, remote) {
      if (err) return callback(err);
      if (res.statusCode !== 200) return callback(new Error('Invalid statusCode returned'));

      // Append get some local additions that are missing from the source
      fs.readFile(exports.after, 'utf8', function reading(err, after) {
        if (err) return callback(err);

        // Parse the contents
        exports.parse([ before, remote, after ], function parsing(err, results, source) {
          callback(err, results);

          if (source && !err) {
            fs.writeFile(exports.output, source, function idk(err) {
              if (err) {
                console.error('Failed to save the generated file due to reasons', err);
              }
            });
          }
        });
      });
    });
  });
};

That is, a failed download or parse doesn't cause the issue.

However, on occasion we do see an empty file. I am wondering if this is a concurrency issue since the nodes docs say:

"Note that it is unsafe to use fs.writeFile multiple times on the same file without waiting for the callback. For this scenario, fs.createWriteStream is strongly recommended."

For example, is it possible that an npm update is in progress at the same time that the updater runs here? https://github.com/3rd-Eden/useragent/blob/master/index.js#L366

(As a side note the comments seem to suggest there is a boolean parameter called refresh to Refresh the dataset from the remote but I don't see such a parameter being passed in or used module.exports = function updater() ).

Another option suggested by a colleague is that the update function could write to a temporary file and make sure there is valid content and only then rename the file.

Thoughts?