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

JSON Formatter: Logs stack trace exception and should log cucumber assertions errors #957

Closed
humphreyn opened this issue Oct 19, 2017 · 12 comments

Comments

@humphreyn
Copy link

Hi,

I think there is a bug in the 'json_formatter.js' at line 213, see snippet below:

    if (status === _status2.default.FAILED && exception) {
      data.result.error_message = exception.stack || exception;
    }

Any failed test will result in the stack trace being logged when IMO it should be the exception.message at a minimum and potentially the stack trace as well (it would be nice if you could turn on/off the stack trace errors).

Can someone confirm if this is a bug and how it will be resolved.

I am running NightwatchJS with CucumberJS, my environment details is as follows:

Node - v8.7.0
Cucumber - v3.03
Nightwatch - v0.9.16
Nightwatch cucumber - v8.2.2
Browser - Chrome v61.0.3163.100
Chrome driver - v2.32
Selenium - v3.5.3
Os - Windows 7

I have attached a copy of json formatter output showing the error message at line 86

Regards,
Neville Humphrey

chrome.cucumber.json.zip

@humphreyn
Copy link
Author

Has no one else seen this bug?

I maintain this is a bug. The line of code in the json_formatter @ 213 should be:

      data.result.error_message = exception.message + exception.stack;

and not

  data.result.error_message = exception.stack || exception;

That way you get the assertion error and the stack trace.

@stevehipwell
Copy link

I've seen the same issue, please can someone confirm that this is a bug?

@charlierudolph
Copy link
Member

charlierudolph commented Nov 2, 2017

It is my experience in chrome and node that Error: <message> is the first line of the stack. Error: <message> is the result of error.toString(). We could use something like:

let msg = exception
if (exception.stack) {
  msg = exception.stack
  if (!_.includes(msg, error.toString()) {
    msg = error.toString() + '\n' + msg
  }
}
data.result.error_message = msg

Can confirm the stack does not include the message in safari. I see the message in the stack in chrome 61 and node 8.

@innocentiv
Copy link
Contributor

I encounter the same problem using nightwatch cucumber (failure will often result in an error message without a stack). It's a bug to me

let error_message = _.includes(exception.stack, exception.message) ? '' : exception.message + '\n';
error_message += exception.stack;
data.result.error_message = error_message || exception;

If the exception message is included in the stack, this solution might provide an error_message without repetition. Otherwise the exception message is put before.

@finaruioana
Copy link

Hello,

We have a similar issue at the same line, but due to the type of this element.
Into the json report file generated, error_message is a string OR an object. Could we have a consistency here? Waiting to hear people's thoughts on it.

The issue for us is when we try to generate the html report from json. At the minute, the cucumber-html-reporter library expects this error_message to be a string. When we get it as an object, the html reporter fails.

Going further, we get this type difference due to tests using different browser libraries. Whenever we run the tests using webdriverio, the report generation passes. But using webdriverjs-angular, the report.json will have the error_message as an object and thus the failure when generating the html report.

Based on your response, I might need to raise the issue in a different place, but I thought the best place to start with this is to see if we could have a consistency over the error_message type.

Thanks!

@humphreyn
Copy link
Author

Hi finaruioana,

In your local version of cucumber/lib/formatter/json_formatter.js could you test if changing line 213 to the following works for you:

      let error_message = _lodash.includes(exception.stack, exception.message) ? '' : exception.message + '\n';
      error_message += exception.stack;
      data.result.error_message = error_message || exception;

If it does I may put in pull request with the above fix, as there does not seem to be any traction on this issue.

@finaruioana
Copy link

finaruioana commented Nov 7, 2017

Hi @humphreyn

Indeed, making the above changes solved my issue. I'd also add an if on the stack before appending it, in my case was appending undefined.

    let error_message = _lodash.includes(exception.stack, exception.message) ? '' : exception.message + '\n';
    if (exception.stack) {
        error_message += exception.stack;
    }
    data.result.error_message = error_message || exception; ```

Thanks! 

@innocentiv
Copy link
Contributor

innocentiv commented Nov 7, 2017

I took for granted that exception is an object with message and stack defined, but if that is not always the case small adjustment are needed:

let msg = exception.message || '';
let stack = exception.stack || '';
msg = _.includes(stack, msg) ? '' : (msg + '\n');
data.result.error_message = (msg + stack) || exception;

This code prevent "undefined" to be added to the error_message if message or stack are not defined. If both are undefined or empty string the exception object is assigned to error_message. If we want to enforce a string error_message we can use JSON.stringify:

let msg = exception.message || '';
let stack = exception.stack || '';
msg = _.includes(stack, msg) ? '' : (msg + '\n');
data.result.error_message = (msg + stack) || JSON.stringify(exception);

@finaruioana you think that this solution would be ok (to test in local environment you have to use _lodash instead of _)

let msg = exception.message || '';
let stack = exception.stack || '';
msg = _lodash.includes(stack, msg) ? '' : (msg + '\n');
data.result.error_message = (msg + stack) || exception;

@finaruioana
Copy link

yeah, looks good @innocentiv. Tested it and seems to work fine.

@humphreyn
Copy link
Author

@innocentiv , I can also confirm that those changes work locally for me in chrome, ie and firefox with the following set up:
nodejs: v8.9.0
cucumberjs: v 3.1.0
nightwatch-cucumber: v0.9.16
selenium-standalone: v3.4.0
ie webdriver: v3.4.0
chrome webdriver: v2.33
gecko (firefox) webdriver: v0.19.1

Will you submit a pull request to apply this fix?

@innocentiv
Copy link
Contributor

@humphreyn Pull request: #973

I just modified the pull request to check for string exception. You can test it locally using:

          if (_lodash2.default.isString(exception)) {
            data.result.error_message = exception;
          } else {
            var _exception$message = exception.message,
                message = _exception$message === undefined ? '' : _exception$message,
                _exception$stack = exception.stack,
                stack = _exception$stack === undefined ? '' : _exception$stack;

            message = _lodash2.default.includes(stack, message) ? '' : message + '\n';
            data.result.error_message = message + stack || JSON.stringify(exception);
          }

@lock
Copy link

lock bot commented Nov 28, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Nov 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants