Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Commit

Permalink
fix(websocket): fix #824, patch websocket onproperties correctly in P…
Browse files Browse the repository at this point in the history 
…hantomJS (#826)

* fix(websocket): fix #824, patch websocket onproperties correctly in phantomjs

* add phantomjs test for local and travis
  • Loading branch information
@JiaLiPassion @mhevery
JiaLiPassion authored and mhevery committed Jul 12, 2017
1 parent 047d9b2 commit 273cb85
Show file tree
Hide file tree
Showing 8 changed files with 162 additions and 118 deletions.
1 change: 1 addition & 0 deletions .travis.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ script:
- node_modules/.bin/gulp build
- scripts/closure/closure_compiler.sh
- node_modules/.bin/gulp promisetest
- npm run test:phantomjs-single
- node_modules/.bin/karma start karma-dist-sauce-jasmine.conf.js --single-run
- node_modules/.bin/karma start karma-build-sauce-mocha.conf.js --single-run
- node_modules/.bin/karma start karma-dist-sauce-selenium3-jasmine.conf.js --single-run
Expand Down
9 changes: 9 additions & 0 deletions karma-build-jasmine-phantomjs.conf.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@

module.exports = function (config) {
require('./karma-build.conf.js')(config);

config.plugins.push(require('karma-jasmine'));
config.plugins.push(require('karma-phantomjs-launcher'));
config.frameworks.push('jasmine');
config.browsers.splice(0, 1, ['PhantomJS']);
};
8 changes: 7 additions & 1 deletion lib/browser/websocket.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,16 @@ export function apply(_global: any) {
const socket = arguments.length > 1 ? new WS(a, b) : new WS(a);
let proxySocket: any;

let proxySocketProto: any;

// Safari 7.0 has non-configurable own 'onmessage' and friends properties on the socket instance
const onmessageDesc = Object.getOwnPropertyDescriptor(socket, 'onmessage');
if (onmessageDesc && onmessageDesc.configurable === false) {
proxySocket = Object.create(socket);
// socket have own property descriptor 'onopen', 'onmessage', 'onclose', 'onerror'
// but proxySocket not, so we will keep socket as prototype and pass it to
// patchOnProperties method
proxySocketProto = socket;
['addEventListener', 'removeEventListener', 'send', 'close'].forEach(function(propName) {
proxySocket[propName] = function() {
return socket[propName].apply(socket, arguments);
Expand All @@ -35,7 +41,7 @@ export function apply(_global: any) {
proxySocket = socket;
}

patchOnProperties(proxySocket, ['close', 'error', 'message', 'open']);
patchOnProperties(proxySocket, ['close', 'error', 'message', 'open'], proxySocketProto);

return proxySocket;
};
Expand Down
6 changes: 6 additions & 0 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@
"closure:test": "scripts/closure/closure_compiler.sh",
"format": "gulp format:enforce",
"karma-jasmine": "karma start karma-build-jasmine.conf.js",
"karma-jasmine:phantomjs": "karma start karma-build-jasmine-phantomjs.conf.js --single-run",
"karma-jasmine:single": "karma start karma-build-jasmine.conf.js --single-run",
"karma-jasmine:autoclose": "npm run karma-jasmine:single && npm run ws-client",
"karma-jasmine-phantomjs:autoclose": "npm run karma-jasmine:phantomjs && npm run ws-client",
"lint": "gulp lint",
"prepublish": "tsc && gulp build",
"promisetest": "gulp promisetest",
Expand All @@ -33,6 +35,8 @@
"tsc": "tsc",
"tsc:w": "tsc -w",
"test": "npm run tsc && concurrently \"npm run tsc:w\" \"npm run ws-server\" \"npm run karma-jasmine\"",
"test:phantomjs": "npm run tsc && concurrently \"npm run tsc:w\" \"npm run ws-server\" \"npm run karma-jasmine:phantomjs\"",
"test:phantomjs-single": "concurrently \"npm run ws-server\" \"npm run karma-jasmine-phantomjs:autoclose\"",
"test:single": "npm run tsc && concurrently \"npm run ws-server\" \"npm run karma-jasmine:autoclose\"",
"test-dist": "concurrently \"npm run tsc:w\" \"npm run ws-server\" \"karma start karma-dist-jasmine.conf.js\"",
"test-node": "gulp test/node",
Expand Down Expand Up @@ -74,11 +78,13 @@
"karma-firefox-launcher": "^0.1.4",
"karma-jasmine": "^0.3.6",
"karma-mocha": "^1.2.0",
"karma-phantomjs-launcher": "^1.0.4",
"karma-safari-launcher": "^0.1.1",
"karma-sauce-launcher": "^0.2.10",
"karma-sourcemap-loader": "^0.3.6",
"mocha": "^3.1.2",
"nodejs-websocket": "^1.2.0",
"phantomjs": "^2.1.7",
"promises-aplus-tests": "^2.1.2",
"pump": "^1.0.1",
"selenium-webdriver": "^3.4.0",
Expand Down
2 changes: 1 addition & 1 deletion test/browser/Notification.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ declare const window: any;
function notificationSupport() {
const desc = window['Notification'] &&
Object.getOwnPropertyDescriptor(window['Notification'].prototype, 'onerror');
return window['Notification'] && window['Notification'].prototype && desc.configurable;
return window['Notification'] && window['Notification'].prototype && desc && desc.configurable;
}

(<any>notificationSupport).message = 'Notification Support';
Expand Down
20 changes: 19 additions & 1 deletion test/test-util.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,22 @@ export function supportPatchXHROnProperty() {
return false;
}
return true;
}
}

let supportSetErrorStack = true;

export function isSupportSetErrorStack() {
try {
throw new Error('test');
} catch (err) {
try {
err.stack = 'new stack';
supportSetErrorStack = err.stack === 'new stack';
} catch (error) {
supportSetErrorStack = false;
}
}
return supportSetErrorStack;
}

(isSupportSetErrorStack as any).message = 'supportSetErrorStack';
227 changes: 115 additions & 112 deletions test/zone-spec/long-stack-trace-zone.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,136 +7,139 @@
*/

import {zoneSymbol} from '../../lib/common/utils';
import {ifEnvSupports, isSupportSetErrorStack} from '../test-util';

const defineProperty = (Object as any)[zoneSymbol('defineProperty')] || Object.defineProperty;

describe('longStackTraceZone', function() {
let log: Error[];
let lstz: Zone;
let longStackTraceZoneSpec = (Zone as any)['longStackTraceZoneSpec'];
describe(
'longStackTraceZone', ifEnvSupports(isSupportSetErrorStack, function() {
let log: Error[];
let lstz: Zone;
let longStackTraceZoneSpec = (Zone as any)['longStackTraceZoneSpec'];

beforeEach(function() {
lstz = Zone.current.fork(longStackTraceZoneSpec).fork({
name: 'long-stack-trace-zone-test',
onHandleError: (parentZoneDelegate: ZoneDelegate, currentZone: Zone, targetZone: Zone,
error: any): boolean => {
parentZoneDelegate.handleError(targetZone, error);
log.push(error);
return false;
}
});
beforeEach(function() {
lstz = Zone.current.fork(longStackTraceZoneSpec).fork({
name: 'long-stack-trace-zone-test',
onHandleError: (parentZoneDelegate: ZoneDelegate, currentZone: Zone, targetZone: Zone,
error: any): boolean => {
parentZoneDelegate.handleError(targetZone, error);
log.push(error);
return false;
}
});

log = [];
});
log = [];
});

function expectElapsed(stack: string, expectedCount: number) {
try {
let actualCount = stack.split('_Elapsed_').length;
if (actualCount !== expectedCount) {
expect(actualCount).toEqual(expectedCount);
console.log(stack);
function expectElapsed(stack: string, expectedCount: number) {
try {
let actualCount = stack.split('_Elapsed_').length;
if (actualCount !== expectedCount) {
expect(actualCount).toEqual(expectedCount);
console.log(stack);
}
} catch (e) {
expect(e).toBe(null);
}
}
} catch (e) {
expect(e).toBe(null);
}
}

it('should produce long stack traces', function(done) {
lstz.run(function() {
setTimeout(function() {
setTimeout(function() {
it('should produce long stack traces', function(done) {
lstz.run(function() {
setTimeout(function() {
expectElapsed(log[0].stack, 3);
done();
setTimeout(function() {
setTimeout(function() {
expectElapsed(log[0].stack, 3);
done();
}, 0);
throw new Error('Hello');
}, 0);
}, 0);
throw new Error('Hello');
}, 0);
}, 0);
});
});
});
});

it('should produce a long stack trace even if stack setter throws', (done) => {
let wasStackAssigned = false;
let error = new Error('Expected error');
defineProperty(error, 'stack', {
configurable: false,
get: () => 'someStackTrace',
set: (v: any) => {
throw new Error('no writes');
}
});
lstz.run(() => {
setTimeout(() => {
throw error;
it('should produce a long stack trace even if stack setter throws', (done) => {
let wasStackAssigned = false;
let error = new Error('Expected error');
defineProperty(error, 'stack', {
configurable: false,
get: () => 'someStackTrace',
set: (v: any) => {
throw new Error('no writes');
}
});
lstz.run(() => {
setTimeout(() => {
throw error;
});
});
setTimeout(() => {
const e = log[0];
expect((e as any).longStack).toBeTruthy();
done();
});
});
});
setTimeout(() => {
const e = log[0];
expect((e as any).longStack).toBeTruthy();
done();
});
});

it('should produce long stack traces when has uncaught error in promise', function(done) {
lstz.runGuarded(function() {
setTimeout(function() {
setTimeout(function() {
let promise = new Promise(function(resolve, reject) {
it('should produce long stack traces when has uncaught error in promise', function(done) {
lstz.runGuarded(function() {
setTimeout(function() {
setTimeout(function() {
reject(new Error('Hello Promise'));
let promise = new Promise(function(resolve, reject) {
setTimeout(function() {
reject(new Error('Hello Promise'));
}, 0);
});
promise.then(function() {
fail('should not get here');
});
setTimeout(function() {
expectElapsed(log[0].stack, 5);
done();
}, 0);
}, 0);
});
promise.then(function() {
fail('should not get here');
});
setTimeout(function() {
expectElapsed(log[0].stack, 5);
done();
}, 0);
}, 0);
}, 0);
});
});
});
});

it('should produce long stack traces when handling error in promise', function(done) {
lstz.runGuarded(function() {
setTimeout(function() {
setTimeout(function() {
let promise = new Promise(function(resolve, reject) {
it('should produce long stack traces when handling error in promise', function(done) {
lstz.runGuarded(function() {
setTimeout(function() {
setTimeout(function() {
try {
throw new Error('Hello Promise');
} catch (err) {
reject(err);
}
let promise = new Promise(function(resolve, reject) {
setTimeout(function() {
try {
throw new Error('Hello Promise');
} catch (err) {
reject(err);
}
}, 0);
});
promise.catch(function(error) {
// should be able to get long stack trace
const longStackFrames: string = longStackTraceZoneSpec.getLongStackTrace(error);
expectElapsed(longStackFrames, 4);
done();
});
}, 0);
});
promise.catch(function(error) {
// should be able to get long stack trace
const longStackFrames: string = longStackTraceZoneSpec.getLongStackTrace(error);
expectElapsed(longStackFrames, 4);
done();
});
}, 0);
}, 0);
});
});
}, 0);
});
});

it('should not produce long stack traces if Error.stackTraceLimit = 0', function(done) {
const originalStackTraceLimit = Error.stackTraceLimit;
lstz.run(function() {
setTimeout(function() {
setTimeout(function() {
it('should not produce long stack traces if Error.stackTraceLimit = 0', function(done) {
const originalStackTraceLimit = Error.stackTraceLimit;
lstz.run(function() {
setTimeout(function() {
if (log[0].stack) {
expectElapsed(log[0].stack, 1);
}
Error.stackTraceLimit = originalStackTraceLimit;
done();
setTimeout(function() {
setTimeout(function() {
if (log[0].stack) {
expectElapsed(log[0].stack, 1);
}
Error.stackTraceLimit = originalStackTraceLimit;
done();
}, 0);
Error.stackTraceLimit = 0;
throw new Error('Hello');
}, 0);
}, 0);
Error.stackTraceLimit = 0;
throw new Error('Hello');
}, 0);
}, 0);
});
});
});
});
});
}));
7 changes: 4 additions & 3 deletions test/zone-spec/task-tracking.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,11 @@ describe('TaskTrackingZone', function() {
xhr.send();
expect(taskTrackingZoneSpec.macroTasks.length).toBe(1);
expect(taskTrackingZoneSpec.macroTasks[0].source).toBe('XMLHttpRequest.send');
expect(taskTrackingZoneSpec.eventTasks[0].source)
.toMatch(/\.addEventListener:readystatechange/);
if (supportPatchXHROnProperty()) {
expect(taskTrackingZoneSpec.eventTasks[0].source)
.toMatch(/\.addEventListener:readystatechange/);
}
});

});
});

Expand Down

0 comments on commit 273cb85

Please sign in to comment.