webdriverio-boneyard / wdio-sync

A WebdriverIO v4 plugin. Helper module to run WebdriverIO commands synchronously.
http://v4.webdriver.io
MIT License
17 stars 31 forks source link

Cannot read property 'split' of undefined #41

Closed khirakawa closed 7 years ago

khirakawa commented 7 years ago

The following test:

browser.addCommand('test', function () {
  throw {
    errorMessage: 'some error'
  };
});

describe('test', () => {
  it('It should work', () => {
    browser.test();
  });
});

Results in a Cannot read property 'split' of undefined error:

[chrome 54.0 #0a] Running: chrome (v54.0)
[chrome 54.0 #0a]
[chrome 54.0 #0a]   test
[chrome 54.0 #0a]       1) it should work
[chrome 54.0 #0a]
[chrome 54.0 #0a]
[chrome 54.0 #0a] 1 failing (5s)
[chrome 54.0 #0a]
[chrome 54.0 #0a] 1) test it should work:
[chrome 54.0 #0a] Cannot read property 'split' of undefined
[chrome 54.0 #0a] TypeError: Cannot read property 'split' of undefined
[chrome 54.0 #0a]     at Context.<anonymous> (/some/path/to/test.spec.js:7:18)

From the stacktrace, it isn't apparent where the error actually occurs. It also swallows the actual error that was thrown. Instead of Cannot read property 'split' of undefined, it would be helpful if the error message was the thrown { errorMessage: 'some error' } object.

(Digging deeper, it looks like the error is thrown at this line. It is trying to parse a stacktrace that doesn't exist because an object was thrown instead of an Error.)

christian-bromann commented 7 years ago

if the error message was the thrown { errorMessage: 'some error' } object.

Please throw the error correctly like:

browser.addCommand('test', function () {
  throw new Error('some error');
});

Then it should work.

khirakawa commented 7 years ago

@christian-bromann I agree that it is common practice to throw a new Error object. However, since any expression can be thrown, I do wonder if other people will also be confused as I was with the Cannot read property 'split' of undefined error.

Here's another example of a test that throws the 'split' error:

browser.addCommand('test', function (func) {
  return func();
});

describe('test', () => {
  it('It should work', () => {
    browser.test.call(null, () => {
      return new Promise((resolve, reject) => {
        reject({});
      });
    });
  });
});

In this case, two things are required for the error to occur:

  1. We are not rejecting a new Error()
  2. We invoke a custom method (browser.test) via .call or .apply

For anyone who stumbles upon this issue, the workaround I found was to not call .call, .apply, or .bind on any custom method. Additionally, reject with an object that has a stack property like new Error as @christian-bromann comments above.

If you use a custom error, make sure to have a stack:

function CustomError (){
  this.stack = ''; // This is required
};
CustomError.prototype = Object.create(Error.prototype);
CustomError.prototype.constructor = CustomError;

Or alternatively, do:

class CustomError extends Error {}
khirakawa commented 7 years ago

@christian-bromann Just realized you made this commit. Nice!