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

patch to include uncaught exceptions to error JSON #1206

Closed
wants to merge 4 commits into from
Closed

patch to include uncaught exceptions to error JSON #1206

wants to merge 4 commits into from

Conversation

ben-bradley
Copy link

I found that uncaught exceptions didn't print the stack, only '{ uncaught: true }' so I borrowed shamelessly from base.js to process & include the stack message. I also cleaned up some more of the comma-firsts

Ben Bradley added 2 commits May 6, 2014 14:31
I found that uncaught exceptions didn't print the stack, only '{ uncaught: true }' so I borrowed shamelessly from base.js to process & include the stack message.  I also cleaned up some more of the comma-firsts
Previous commit had unnecessary code & didn't print full stack trace.
@tj
Copy link
Contributor

tj commented May 16, 2014

why change the names? .message and .stack would be better, and without the unrelated style changes would be rad

@ben-bradley
Copy link
Author

With a test file like this:

var should = require('should'),
    request = require('request');

describe('simple POST test', function() {
  it('should NOT work', function(done) {
    request.post('http://www.google.com', function(err, res, body) {
      blargh(); // throws uncaught error
      (res.statusCode).should.equal(200);
      done();
    });
  });
});

The blargh(); function throws an uncaught error and all the Error properties (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error#Properties_2) get stripped off of test.err. By renaming them, curiously, they make it through to stdout.

We can keep the names, however, by re-initing the test.err object like this:

// line 38
  runner.on('fail', function(test){
    if (test.err.uncaught && !test.err.generatedMessage) {
      var error = {},
          props = [ 'stack', 'message', 'name', 'fileName', 'lineNumber' ];
      props.forEach(function(prop) {
        error[prop] = test.err[prop];
      });
      test.err = error;
    }
    failures.push(test);
  });

@jbnicolai
Copy link

@ben-bradley if you could revert the style changes in lines 6-8 and 27-29 I would be happy to merge this.

@parkr
Copy link
Contributor

parkr commented Aug 26, 2014

@jbnicolai Looks like the style changes have been reverted.

@ben-bradley
Copy link
Author

@parkr & @jbnicolai,

It looks like #1295 made some changes that conflict with this one. The solution provided in #1295 is cleaner than this one so I'd go with that, though it did introduce a bug, but #1324 should fix it though.

-Ben

@jbnicolai
Copy link

@ben-bradley thanks for your patience and your work on this! Looking at #1324 now.

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

Successfully merging this pull request may close these issues.

4 participants