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.

Sign up for GitHub

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

Jump to bottom

Fix bugs for 1.0.2 #369

Merged
merged 13 commits into from
Feb 21, 2018
Merged

Fix bugs for 1.0.2 #369

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
c1e77f3
Fix issue #362
OsvaldoRosado Feb 1, 2018
4376ce8
Fix issue #358
OsvaldoRosado Feb 1, 2018
63ce32b
Fix issue #346
OsvaldoRosado Feb 2, 2018
d7f737f
Fix issue #344
OsvaldoRosado Feb 3, 2018
b324e79
Address issue #368
OsvaldoRosado Feb 7, 2018
0ce908d
Address issue #365
OsvaldoRosado Feb 7, 2018
76f8a8d
Add additional logging and fix stability issues with retry caching
OsvaldoRosado Feb 8, 2018
2824d58
Bump package.json
OsvaldoRosado Feb 8, 2018
f944e1a
Add calls to ICACLS for windows
OsvaldoRosado Feb 20, 2018
6b3abbb
Address PR feedback
OsvaldoRosado Feb 20, 2018
be77e73
Fix unit tests in Node < 0.11.12
OsvaldoRosado Feb 21, 2018
a2456db
Fix more unit tests for Node < 0.11.12
OsvaldoRosado Feb 21, 2018
4b61c5c
Fix again
OsvaldoRosado Feb 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 14 additions & 9 deletions AutoCollection/CorrelationContextManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ export class CorrelationContextManager {
if ((<any>orig).prepareStackTrace) {
(<any>orig).stackRewrite= false;
var stackTrace = (<any>orig).prepareStackTrace;
(<any>orig).prepareStackTrace = (e: any, s: any) => {
(<any>orig).prepareStackTrace = (e: any, s: any[]) => {
// Remove some AI and Zone methods from the stack trace
// Otherwise we leave side-effects

Expand All @@ -203,20 +203,25 @@ export class CorrelationContextManager {
// Zone | Zone | CorrelationContextManager | CorrelationContextManager | User
var foundOne = false;
for (var i=0; i<s.length; i++) {
if (s[i].getFileName().indexOf("AutoCollection/CorrelationContextManager") === -1 &&
s[i].getFileName().indexOf("AutoCollection\\CorrelationContextManager") === -1) {

if (foundOne) {
break;
let fileName = s[i] && typeof s[i].getFileName === 'function' && s[i].getFileName();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition could be simplified to const fileName = s[i].getFileName(). getFileName should be always present on a CallSite object. We only need to check that filename is returned (in our case we seen undefined for functions defined inside eval).

https://github.com/v8/v8/wiki/Stack-Trace-API

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Thanks!

if (fileName) {
if (fileName.indexOf("AutoCollection/CorrelationContextManager") === -1 &&
fileName.indexOf("AutoCollection\\CorrelationContextManager") === -1) {

if (foundOne) {
break;
}
} else {
foundOne = true;
}
} else {
foundOne = true;
}
}
// Loop above goes one extra step
i = Math.max(0, i - 1);

s.splice(0, i);
if (foundOne) {
s.splice(0, i);
}
return stackTrace(e, s);
}
}
Expand Down
17 changes: 17 additions & 0 deletions Tests/AutoCollection/CorrelationContextManager.tests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,23 @@ if (CorrelationContextManager.isNodeVersionCompatible()) {
var topOfStack = (<any>error.stack)[0].getFileName();
assert(topOfStack.indexOf("CorrelationContextManager.tests.js") !== -1, "Top of stack not expected to be " + topOfStack);
});
it("should not crash on missing filename", () => {
CorrelationContextManager.enable();

var stackTrace = (<any>Error)['prepareStackTrace'];
(<any>Error)['prepareStackTrace'] = function (_: any, stack: any): any[] {
return stack;
};

var error = new Error();
try {
(<any>Error)['prepareStackTrace'](null, [{getFunctionName: ()=>''}]);
(<any>Error)['prepareStackTrace'] = stackTrace;
} catch (e) {
(<any>Error)['prepareStackTrace'] = stackTrace;
assert(false, "prepareStackTrace should not throw. Threw: " + e);
}
});
});

describe("#runWithContext()", () => {
Expand Down