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

Commit 273cb85

Browse files
JiaLiPassionmhevery
JiaLiPassion
authored and
mhevery
committed
fix(websocket): fix #824, patch websocket onproperties correctly in PhantomJS (#826)
* fix(websocket): fix #824, patch websocket onproperties correctly in phantomjs * add phantomjs test for local and travis
1 parent 047d9b2 commit 273cb85

8 files changed

Lines changed: 162 additions & 118 deletions

File tree

‎.travis.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ script:
2727
- node_modules/.bin/gulp build
2828
- scripts/closure/closure_compiler.sh
2929
- node_modules/.bin/gulp promisetest
30+
- npm run test:phantomjs-single
3031
- node_modules/.bin/karma start karma-dist-sauce-jasmine.conf.js --single-run
3132
- node_modules/.bin/karma start karma-build-sauce-mocha.conf.js --single-run
3233
- node_modules/.bin/karma start karma-dist-sauce-selenium3-jasmine.conf.js --single-run

‎karma-build-jasmine-phantomjs.conf.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
2+
module.exports = function (config) {
3+
require('./karma-build.conf.js')(config);
4+
5+
config.plugins.push(require('karma-jasmine'));
6+
config.plugins.push(require('karma-phantomjs-launcher'));
7+
config.frameworks.push('jasmine');
8+
config.browsers.splice(0, 1, ['PhantomJS']);
9+
};

‎lib/browser/websocket.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,16 @@ export function apply(_global: any) {
2121
const socket = arguments.length > 1 ? new WS(a, b) : new WS(a);
2222
let proxySocket: any;
2323

24+
let proxySocketProto: any;
25+
2426
// Safari 7.0 has non-configurable own 'onmessage' and friends properties on the socket instance
2527
const onmessageDesc = Object.getOwnPropertyDescriptor(socket, 'onmessage');
2628
if (onmessageDesc && onmessageDesc.configurable === false) {
2729
proxySocket = Object.create(socket);
30+
// socket have own property descriptor 'onopen', 'onmessage', 'onclose', 'onerror'
31+
// but proxySocket not, so we will keep socket as prototype and pass it to
32+
// patchOnProperties method
33+
proxySocketProto = socket;
2834
['addEventListener', 'removeEventListener', 'send', 'close'].forEach(function(propName) {
2935
proxySocket[propName] = function() {
3036
return socket[propName].apply(socket, arguments);
@@ -35,7 +41,7 @@ export function apply(_global: any) {
3541
proxySocket = socket;
3642
}
3743

38-
patchOnProperties(proxySocket, ['close', 'error', 'message', 'open']);
44+
patchOnProperties(proxySocket, ['close', 'error', 'message', 'open'], proxySocketProto);
3945

4046
return proxySocket;
4147
};

‎package.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,10 @@
1919
"closure:test": "scripts/closure/closure_compiler.sh",
2020
"format": "gulp format:enforce",
2121
"karma-jasmine": "karma start karma-build-jasmine.conf.js",
22+
"karma-jasmine:phantomjs": "karma start karma-build-jasmine-phantomjs.conf.js --single-run",
2223
"karma-jasmine:single": "karma start karma-build-jasmine.conf.js --single-run",
2324
"karma-jasmine:autoclose": "npm run karma-jasmine:single && npm run ws-client",
25+
"karma-jasmine-phantomjs:autoclose": "npm run karma-jasmine:phantomjs && npm run ws-client",
2426
"lint": "gulp lint",
2527
"prepublish": "tsc && gulp build",
2628
"promisetest": "gulp promisetest",
@@ -33,6 +35,8 @@
3335
"tsc": "tsc",
3436
"tsc:w": "tsc -w",
3537
"test": "npm run tsc && concurrently \"npm run tsc:w\" \"npm run ws-server\" \"npm run karma-jasmine\"",
38+
"test:phantomjs": "npm run tsc && concurrently \"npm run tsc:w\" \"npm run ws-server\" \"npm run karma-jasmine:phantomjs\"",
39+
"test:phantomjs-single": "concurrently \"npm run ws-server\" \"npm run karma-jasmine-phantomjs:autoclose\"",
3640
"test:single": "npm run tsc && concurrently \"npm run ws-server\" \"npm run karma-jasmine:autoclose\"",
3741
"test-dist": "concurrently \"npm run tsc:w\" \"npm run ws-server\" \"karma start karma-dist-jasmine.conf.js\"",
3842
"test-node": "gulp test/node",
@@ -74,11 +78,13 @@
7478
"karma-firefox-launcher": "^0.1.4",
7579
"karma-jasmine": "^0.3.6",
7680
"karma-mocha": "^1.2.0",
81+
"karma-phantomjs-launcher": "^1.0.4",
7782
"karma-safari-launcher": "^0.1.1",
7883
"karma-sauce-launcher": "^0.2.10",
7984
"karma-sourcemap-loader": "^0.3.6",
8085
"mocha": "^3.1.2",
8186
"nodejs-websocket": "^1.2.0",
87+
"phantomjs": "^2.1.7",
8288
"promises-aplus-tests": "^2.1.2",
8389
"pump": "^1.0.1",
8490
"selenium-webdriver": "^3.4.0",

‎test/browser/Notification.spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ declare const window: any;
1515
function notificationSupport() {
1616
const desc = window['Notification'] &&
1717
Object.getOwnPropertyDescriptor(window['Notification'].prototype, 'onerror');
18-
return window['Notification'] && window['Notification'].prototype && desc.configurable;
18+
return window['Notification'] && window['Notification'].prototype && desc && desc.configurable;
1919
}
2020

2121
(<any>notificationSupport).message = 'Notification Support';

‎test/test-util.ts‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,4 +67,22 @@ export function supportPatchXHROnProperty() {
6767
return false;
6868
}
6969
return true;
70-
}
70+
}
71+
72+
let supportSetErrorStack = true;
73+
74+
export function isSupportSetErrorStack() {
75+
try {
76+
throw new Error('test');
77+
} catch (err) {
78+
try {
79+
err.stack = 'new stack';
80+
supportSetErrorStack = err.stack === 'new stack';
81+
} catch (error) {
82+
supportSetErrorStack = false;
83+
}
84+
}
85+
return supportSetErrorStack;
86+
}
87+
88+
(isSupportSetErrorStack as any).message = 'supportSetErrorStack';

‎test/zone-spec/long-stack-trace-zone.spec.ts‎

Lines changed: 115 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -7,136 +7,139 @@
77
*/
88

99
import {zoneSymbol} from '../../lib/common/utils';
10+
import {ifEnvSupports, isSupportSetErrorStack} from '../test-util';
11+
1012
const defineProperty = (Object as any)[zoneSymbol('defineProperty')] || Object.defineProperty;
1113

12-
describe('longStackTraceZone', function() {
13-
let log: Error[];
14-
let lstz: Zone;
15-
let longStackTraceZoneSpec = (Zone as any)['longStackTraceZoneSpec'];
14+
describe(
15+
'longStackTraceZone', ifEnvSupports(isSupportSetErrorStack, function() {
16+
let log: Error[];
17+
let lstz: Zone;
18+
let longStackTraceZoneSpec = (Zone as any)['longStackTraceZoneSpec'];
1619

17-
beforeEach(function() {
18-
lstz = Zone.current.fork(longStackTraceZoneSpec).fork({
19-
name: 'long-stack-trace-zone-test',
20-
onHandleError: (parentZoneDelegate: ZoneDelegate, currentZone: Zone, targetZone: Zone,
21-
error: any): boolean => {
22-
parentZoneDelegate.handleError(targetZone, error);
23-
log.push(error);
24-
return false;
25-
}
26-
});
20+
beforeEach(function() {
21+
lstz = Zone.current.fork(longStackTraceZoneSpec).fork({
22+
name: 'long-stack-trace-zone-test',
23+
onHandleError: (parentZoneDelegate: ZoneDelegate, currentZone: Zone, targetZone: Zone,
24+
error: any): boolean => {
25+
parentZoneDelegate.handleError(targetZone, error);
26+
log.push(error);
27+
return false;
28+
}
29+
});
2730

28-
log = [];
29-
});
31+
log = [];
32+
});
3033

31-
function expectElapsed(stack: string, expectedCount: number) {
32-
try {
33-
let actualCount = stack.split('_Elapsed_').length;
34-
if (actualCount !== expectedCount) {
35-
expect(actualCount).toEqual(expectedCount);
36-
console.log(stack);
34+
function expectElapsed(stack: string, expectedCount: number) {
35+
try {
36+
let actualCount = stack.split('_Elapsed_').length;
37+
if (actualCount !== expectedCount) {
38+
expect(actualCount).toEqual(expectedCount);
39+
console.log(stack);
40+
}
41+
} catch (e) {
42+
expect(e).toBe(null);
43+
}
3744
}
38-
} catch (e) {
39-
expect(e).toBe(null);
40-
}
41-
}
4245

43-
it('should produce long stack traces', function(done) {
44-
lstz.run(function() {
45-
setTimeout(function() {
46-
setTimeout(function() {
46+
it('should produce long stack traces', function(done) {
47+
lstz.run(function() {
4748
setTimeout(function() {
48-
expectElapsed(log[0].stack, 3);
49-
done();
49+
setTimeout(function() {
50+
setTimeout(function() {
51+
expectElapsed(log[0].stack, 3);
52+
done();
53+
}, 0);
54+
throw new Error('Hello');
55+
}, 0);
5056
}, 0);
51-
throw new Error('Hello');
52-
}, 0);
53-
}, 0);
54-
});
55-
});
57+
});
58+
});
5659

57-
it('should produce a long stack trace even if stack setter throws', (done) => {
58-
let wasStackAssigned = false;
59-
let error = new Error('Expected error');
60-
defineProperty(error, 'stack', {
61-
configurable: false,
62-
get: () => 'someStackTrace',
63-
set: (v: any) => {
64-
throw new Error('no writes');
65-
}
66-
});
67-
lstz.run(() => {
68-
setTimeout(() => {
69-
throw error;
60+
it('should produce a long stack trace even if stack setter throws', (done) => {
61+
let wasStackAssigned = false;
62+
let error = new Error('Expected error');
63+
defineProperty(error, 'stack', {
64+
configurable: false,
65+
get: () => 'someStackTrace',
66+
set: (v: any) => {
67+
throw new Error('no writes');
68+
}
69+
});
70+
lstz.run(() => {
71+
setTimeout(() => {
72+
throw error;
73+
});
74+
});
75+
setTimeout(() => {
76+
const e = log[0];
77+
expect((e as any).longStack).toBeTruthy();
78+
done();
79+
});
7080
});
71-
});
72-
setTimeout(() => {
73-
const e = log[0];
74-
expect((e as any).longStack).toBeTruthy();
75-
done();
76-
});
77-
});
7881

79-
it('should produce long stack traces when has uncaught error in promise', function(done) {
80-
lstz.runGuarded(function() {
81-
setTimeout(function() {
82-
setTimeout(function() {
83-
let promise = new Promise(function(resolve, reject) {
82+
it('should produce long stack traces when has uncaught error in promise', function(done) {
83+
lstz.runGuarded(function() {
84+
setTimeout(function() {
8485
setTimeout(function() {
85-
reject(new Error('Hello Promise'));
86+
let promise = new Promise(function(resolve, reject) {
87+
setTimeout(function() {
88+
reject(new Error('Hello Promise'));
89+
}, 0);
90+
});
91+
promise.then(function() {
92+
fail('should not get here');
93+
});
94+
setTimeout(function() {
95+
expectElapsed(log[0].stack, 5);
96+
done();
97+
}, 0);
8698
}, 0);
87-
});
88-
promise.then(function() {
89-
fail('should not get here');
90-
});
91-
setTimeout(function() {
92-
expectElapsed(log[0].stack, 5);
93-
done();
9499
}, 0);
95-
}, 0);
96-
}, 0);
97-
});
98-
});
100+
});
101+
});
99102

100-
it('should produce long stack traces when handling error in promise', function(done) {
101-
lstz.runGuarded(function() {
102-
setTimeout(function() {
103-
setTimeout(function() {
104-
let promise = new Promise(function(resolve, reject) {
103+
it('should produce long stack traces when handling error in promise', function(done) {
104+
lstz.runGuarded(function() {
105+
setTimeout(function() {
105106
setTimeout(function() {
106-
try {
107-
throw new Error('Hello Promise');
108-
} catch (err) {
109-
reject(err);
110-
}
107+
let promise = new Promise(function(resolve, reject) {
108+
setTimeout(function() {
109+
try {
110+
throw new Error('Hello Promise');
111+
} catch (err) {
112+
reject(err);
113+
}
114+
}, 0);
115+
});
116+
promise.catch(function(error) {
117+
// should be able to get long stack trace
118+
const longStackFrames: string = longStackTraceZoneSpec.getLongStackTrace(error);
119+
expectElapsed(longStackFrames, 4);
120+
done();
121+
});
111122
}, 0);
112-
});
113-
promise.catch(function(error) {
114-
// should be able to get long stack trace
115-
const longStackFrames: string = longStackTraceZoneSpec.getLongStackTrace(error);
116-
expectElapsed(longStackFrames, 4);
117-
done();
118-
});
119-
}, 0);
120-
}, 0);
121-
});
122-
});
123+
}, 0);
124+
});
125+
});
123126

124-
it('should not produce long stack traces if Error.stackTraceLimit = 0', function(done) {
125-
const originalStackTraceLimit = Error.stackTraceLimit;
126-
lstz.run(function() {
127-
setTimeout(function() {
128-
setTimeout(function() {
127+
it('should not produce long stack traces if Error.stackTraceLimit = 0', function(done) {
128+
const originalStackTraceLimit = Error.stackTraceLimit;
129+
lstz.run(function() {
129130
setTimeout(function() {
130-
if (log[0].stack) {
131-
expectElapsed(log[0].stack, 1);
132-
}
133-
Error.stackTraceLimit = originalStackTraceLimit;
134-
done();
131+
setTimeout(function() {
132+
setTimeout(function() {
133+
if (log[0].stack) {
134+
expectElapsed(log[0].stack, 1);
135+
}
136+
Error.stackTraceLimit = originalStackTraceLimit;
137+
done();
138+
}, 0);
139+
Error.stackTraceLimit = 0;
140+
throw new Error('Hello');
141+
}, 0);
135142
}, 0);
136-
Error.stackTraceLimit = 0;
137-
throw new Error('Hello');
138-
}, 0);
139-
}, 0);
140-
});
141-
});
142-
});
143+
});
144+
});
145+
}));

‎test/zone-spec/task-tracking.spec.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,11 @@ describe('TaskTrackingZone', function() {
6161
xhr.send();
6262
expect(taskTrackingZoneSpec.macroTasks.length).toBe(1);
6363
expect(taskTrackingZoneSpec.macroTasks[0].source).toBe('XMLHttpRequest.send');
64-
expect(taskTrackingZoneSpec.eventTasks[0].source)
65-
.toMatch(/\.addEventListener:readystatechange/);
64+
if (supportPatchXHROnProperty()) {
65+
expect(taskTrackingZoneSpec.eventTasks[0].source)
66+
.toMatch(/\.addEventListener:readystatechange/);
67+
}
6668
});
67-
6869
});
6970
});
7071

0 commit comments

Comments
 (0)