Update utils.js

[stambata]

Please consider the proposed change.The source code of the non-native stacktrace lines was never fetched on Windows machines. That's because isInternal was always resolving to true no matter what:

isInternal = line.isNative() || (frame.filename[0] !== '/' && frame.filename[0] !== '.'); 

I added 2 small modifications:

1. the hardcoded separator ('/') has been replaced with path.sep (please refer to: https://nodejs.org/api/path.html#path_path_sep). That way the check is valid for all platforms: '/' for linux and '' for windows
2. an extra check has been added: frame.filename.indexOf(':') != 1
   That way absolute paths on windows are considered non-native.

[mattrobenolt]

Is this supposed to be :\\ or :\? Also, can you switch it to be a strict inequality check? !==

[stambata]

Yes, it's just one backslash but it has to be escaped with another one inside the string. I am now changing the inequality check.

[mattrobenolt]

Oh, derp. You're right. :)

[mattrobenolt]

Is it reasonable to ask for a test or so to cover this case since I never think about Windows, and would want to prevent a regression?

[stambata]

The requested test case has been committed.

[mattrobenolt]

How is this test testing the new code that was just added? I don't see anything about looking for :\ in the frame or a \ at all. Just a bit confused on what this is attempting to test.

[stambata]

Well, actually the test does much more than that. Here it is what my point was when writing it:

1. A real js exception is caused on purpose inside the try block
2. It gets passed to parseStack along with a custom callback knowing that the callback gets invoked with an array of all the processed frames once the whole stack is parsed.
3. As the frames array gets reversed inside parseStack I know that the last frame corresponds to the file where the exception was generated and that the exception isn't inside native code.
4. I verify that the frame has properties: pre_context, context_line and post_context which is valid for non-internal stack lines only.
   The test was failing without the change as inside parseStack the following was always resolving to true:

isInternal = line.isNative() || (frame.filename[0] !== '/' && frame.filename[0] !== '.');

and the frame was never having pre_context, context_line and post_context

[mattrobenolt]

But to touch the branch that you added, that mandates that this test is run on Windows, correct? If the test is run on anything non-windows, your code branches never get touched. So I agree the test is useful, we just need to mock it to force a windows style frame.filename so that we can assert it does the right thing. Especially since all testing is run in Linux or OSX.

[stambata]

Alright, I will think about forcing a windows-style frame.filename and will commit again once ready. But that'll be tomorrow as here in Europe it is beer time now.

[benvinegar]

@stambata – hey just checking in. Any chance you can update the test so we can accept this PR?

[seanbriceland]

@benvinegar & @stambata, Would love to see this get in so i don't have to fork such a minimal change. This could be terribly wrong, but here is a stab at an IT without mocking/patching the filenames under test:

it('should treat windows files as being in app: in_app should be true', function(done){
    var parseStack = raven.utils.parseStack;
    var callback = function(frames) {
        var frame = frames.pop();
        frame.filename.should.be.type('string');
        frame.filename.should.startWith('C:\\');
        frame.in_app.should.be.true;
        done();
    };
    var err = new Error('some error message');
    // get first line of err stack (line after err message)
    var firstFileLine = err.stack.split('\n')[1];
    // replace first occurrence of "/" with C:\ to mock windows style
    var winFirstFileLine = firstFileLine.replace(/[\/]/, 'C:\\');
    // replace all remaining "/" with "\"
    winFirstFileLine = winFirstFileLine.replace(/[\/]/g, '\\');

    // create a "win-style" stack replacing the first err.stack line with our above win-style line
    err.stack = err.stack.replace(firstFileLine, winFirstFileLine);
    parseStack(err, callback);

});

[MaxBittker]

moved this to #268