Skip to content

Commit bcceb35

Browse files
kinyoklionkinyoklion
authored
fix: Ensure that each request can only fail once. (#28)
On WSL with no network, or running docker without a network, the 'error' and 'timeout' callbacks would both be called. The timeout should only happen after the specified timeout, 5 minutes in the node SDK, but happens immediately under either of those conditions. This change ensures that each request can only fail one time.
1 parent ada7031 commit bcceb35

File tree

2 files changed

+110
-62
lines changed

2 files changed

+110
-62
lines changed

‎example/eventsource-polyfill.js‎

Lines changed: 78 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -67,27 +67,27 @@
6767
/* 0 */
6868
/***/ (function(module, exports) {
6969

70-
var g;
71-
72-
// This works in non-strict mode
73-
g = (function() {
74-
return this;
75-
})();
76-
77-
try {
78-
// This works if eval is allowed (see CSP)
79-
g = g || Function("return this")() || (1,eval)("this");
80-
} catch(e) {
81-
// This works if the window reference is available
82-
if(typeof window === "object")
83-
g = window;
84-
}
85-
86-
// g can still be undefined, but nothing to do about it...
87-
// We return undefined, instead of nothing here, so it's
88-
// easier to handle this case. if(!global) { ...}
89-
90-
module.exports = g;
70+
var g;
71+
72+
// This works in non-strict mode
73+
g = (function() {
74+
return this;
75+
})();
76+
77+
try {
78+
// This works if eval is allowed (see CSP)
79+
g = g || Function("return this")() || (1,eval)("this");
80+
} catch(e) {
81+
// This works if the window reference is available
82+
if(typeof window === "object")
83+
g = window;
84+
}
85+
86+
// g can still be undefined, but nothing to do about it...
87+
// We return undefined, instead of nothing here, so it's
88+
// easier to handle this case. if(!global) { ...}
89+
90+
module.exports = g;
9191

9292

9393
/***/ }),
@@ -4675,7 +4675,7 @@ var IncomingMessage = exports.IncomingMessage = function (xhr, response, mode, f
46754675
self.url = response.url
46764676
self.statusCode = response.status
46774677
self.statusMessage = response.statusText
4678-
4678+
46794679
response.headers.forEach(function (header, key){
46804680
self.headers[key.toLowerCase()] = header
46814681
self.rawHeaders.push(key, header)
@@ -4805,7 +4805,7 @@ IncomingMessage.prototype._onXHRProgress = function () {
48054805
self.push(new Buffer(response))
48064806
break
48074807
}
4808-
// Falls through in IE8
4808+
// Falls through in IE8
48094809
case 'text':
48104810
try { // This will fail when readyState = 3 in IE9. Switch mode and wait for readyState = 4
48114811
response = xhr.responseText
@@ -7264,6 +7264,19 @@ function hasBom (buf) {
72647264
})
72657265
}
72667266

7267+
/**
7268+
* Wrap a callback to ensure it can only be called once.
7269+
*/
7270+
function once(cb) {
7271+
let called = false
7272+
return (...params) => {
7273+
if(!called) {
7274+
called = true
7275+
cb(...params)
7276+
}
7277+
}
7278+
}
7279+
72677280
/**
72687281
* Creates a new EventSource object
72697282
*
@@ -7423,17 +7436,25 @@ function EventSource (url, eventSourceInitDict) {
74237436
}, delay)
74247437
}
74257438

7439+
function destroyRequest() {
7440+
if (req.destroy) req.destroy()
7441+
if (req.xhr && req.xhr.abort) req.xhr.abort()
7442+
}
7443+
74267444
function connect () {
74277445
var urlAndOptions = makeRequestUrlAndOptions()
74287446
var isSecure = urlAndOptions.options.protocol === 'https:' ||
74297447
(urlAndOptions.url && urlAndOptions.url.startsWith('https:'))
74307448

7449+
// Each request should be able to fail at most once.
7450+
const failOnce = once(failed)
7451+
74317452
var callback = function (res) {
74327453
// Handle HTTP redirects
74337454
if (res.statusCode === 301 || res.statusCode === 307) {
74347455
if (!res.headers.location) {
74357456
// Server sent redirect response without Location header.
7436-
failed({ status: res.statusCode, message: res.statusMessage })
7457+
failOnce({ status: res.statusCode, message: res.statusMessage })
74377458
return
74387459
}
74397460
if (res.statusCode === 307) reconnectUrl = url
@@ -7444,7 +7465,7 @@ function EventSource (url, eventSourceInitDict) {
74447465

74457466
// Handle HTTP errors
74467467
if (res.statusCode !== 200) {
7447-
failed({ status: res.statusCode, message: res.statusMessage })
7468+
failOnce({ status: res.statusCode, message: res.statusMessage })
74487469
return
74497470
}
74507471

@@ -7456,13 +7477,13 @@ function EventSource (url, eventSourceInitDict) {
74567477
res.on('close', function () {
74577478
res.removeAllListeners('close')
74587479
res.removeAllListeners('end')
7459-
failed()
7480+
failOnce()
74607481
})
74617482

74627483
res.on('end', function () {
74637484
res.removeAllListeners('close')
74647485
res.removeAllListeners('end')
7465-
failed()
7486+
failOnce()
74667487
})
74677488
_emit(new Event('open'))
74687489

@@ -7561,12 +7582,14 @@ function EventSource (url, eventSourceInitDict) {
75617582
}
75627583

75637584
req.on('error', function (err) {
7564-
failed({ message: err.message })
7585+
failOnce({ message: err.message })
75657586
})
75667587

75677588
req.on('timeout', function () {
7568-
failed({ message: 'Read timeout, received no data in ' + config.readTimeoutMillis +
7589+
failOnce({ message: 'Read timeout, received no data in ' + config.readTimeoutMillis +
75697590
'ms, assuming connection is dead' })
7591+
// Timeout doesn't mean that the request is cancelled, just that it has elapsed the timeout.
7592+
destroyRequest()
75707593
})
75717594

75727595
if (req.setNoDelay) req.setNoDelay(true)
@@ -7584,8 +7607,9 @@ function EventSource (url, eventSourceInitDict) {
75847607
this._close = function () {
75857608
if (readyState === EventSource.CLOSED) return
75867609
readyState = EventSource.CLOSED
7587-
if (req.abort) req.abort()
7588-
if (req.xhr && req.xhr.abort) req.xhr.abort()
7610+
7611+
destroyRequest()
7612+
75897613
_emit(new Event('closed'))
75907614
}
75917615

@@ -8711,28 +8735,28 @@ module.exports = CalculateCapacity
87118735
/* 33 */
87128736
/***/ (function(module, exports) {
87138737

8714-
module.exports = function(module) {
8715-
if(!module.webpackPolyfill) {
8716-
module.deprecate = function() {};
8717-
module.paths = [];
8718-
// module.parent = undefined by default
8719-
if(!module.children) module.children = [];
8720-
Object.defineProperty(module, "loaded", {
8721-
enumerable: true,
8722-
get: function() {
8723-
return module.l;
8724-
}
8725-
});
8726-
Object.defineProperty(module, "id", {
8727-
enumerable: true,
8728-
get: function() {
8729-
return module.i;
8730-
}
8731-
});
8732-
module.webpackPolyfill = 1;
8733-
}
8734-
return module;
8735-
};
8738+
module.exports = function(module) {
8739+
if(!module.webpackPolyfill) {
8740+
module.deprecate = function() {};
8741+
module.paths = [];
8742+
// module.parent = undefined by default
8743+
if(!module.children) module.children = [];
8744+
Object.defineProperty(module, "loaded", {
8745+
enumerable: true,
8746+
get: function() {
8747+
return module.l;
8748+
}
8749+
});
8750+
Object.defineProperty(module, "id", {
8751+
enumerable: true,
8752+
get: function() {
8753+
return module.i;
8754+
}
8755+
});
8756+
module.webpackPolyfill = 1;
8757+
}
8758+
return module;
8759+
};
87368760

87378761

87388762
/***/ }),
@@ -12056,4 +12080,4 @@ module.exports = function isBuffer(arg) {
1205612080
}
1205712081

1205812082
/***/ })
12059-
/******/ ]);
12083+
/******/ ]);

‎lib/eventsource.js‎

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,19 @@ function hasBom (buf) {
2727
})
2828
}
2929

30+
/**
31+
* Wrap a callback to ensure it can only be called once.
32+
*/
33+
function once(cb) {
34+
let called = false
35+
return (...params) => {
36+
if(!called) {
37+
called = true
38+
cb(...params)
39+
}
40+
}
41+
}
42+
3043
/**
3144
* Creates a new EventSource object
3245
*
@@ -186,17 +199,25 @@ function EventSource (url, eventSourceInitDict) {
186199
}, delay)
187200
}
188201

202+
function destroyRequest() {
203+
if (req.destroy) req.destroy()
204+
if (req.xhr && req.xhr.abort) req.xhr.abort()
205+
}
206+
189207
function connect () {
190208
var urlAndOptions = makeRequestUrlAndOptions()
191209
var isSecure = urlAndOptions.options.protocol === 'https:' ||
192210
(urlAndOptions.url && urlAndOptions.url.startsWith('https:'))
193211

212+
// Each request should be able to fail at most once.
213+
const failOnce = once(failed)
214+
194215
var callback = function (res) {
195216
// Handle HTTP redirects
196217
if (res.statusCode === 301 || res.statusCode === 307) {
197218
if (!res.headers.location) {
198219
// Server sent redirect response without Location header.
199-
failed({ status: res.statusCode, message: res.statusMessage })
220+
failOnce({ status: res.statusCode, message: res.statusMessage })
200221
return
201222
}
202223
if (res.statusCode === 307) reconnectUrl = url
@@ -207,7 +228,7 @@ function EventSource (url, eventSourceInitDict) {
207228

208229
// Handle HTTP errors
209230
if (res.statusCode !== 200) {
210-
failed({ status: res.statusCode, message: res.statusMessage })
231+
failOnce({ status: res.statusCode, message: res.statusMessage })
211232
return
212233
}
213234

@@ -219,13 +240,13 @@ function EventSource (url, eventSourceInitDict) {
219240
res.on('close', function () {
220241
res.removeAllListeners('close')
221242
res.removeAllListeners('end')
222-
failed()
243+
failOnce()
223244
})
224245

225246
res.on('end', function () {
226247
res.removeAllListeners('close')
227248
res.removeAllListeners('end')
228-
failed()
249+
failOnce()
229250
})
230251
_emit(new Event('open'))
231252

@@ -324,12 +345,14 @@ function EventSource (url, eventSourceInitDict) {
324345
}
325346

326347
req.on('error', function (err) {
327-
failed({ message: err.message })
348+
failOnce({ message: err.message })
328349
})
329350

330351
req.on('timeout', function () {
331-
failed({ message: 'Read timeout, received no data in ' + config.readTimeoutMillis +
352+
failOnce({ message: 'Read timeout, received no data in ' + config.readTimeoutMillis +
332353
'ms, assuming connection is dead' })
354+
// Timeout doesn't mean that the request is cancelled, just that it has elapsed the timeout.
355+
destroyRequest()
333356
})
334357

335358
if (req.setNoDelay) req.setNoDelay(true)
@@ -347,8 +370,9 @@ function EventSource (url, eventSourceInitDict) {
347370
this._close = function () {
348371
if (readyState === EventSource.CLOSED) return
349372
readyState = EventSource.CLOSED
350-
if (req.abort) req.abort()
351-
if (req.xhr && req.xhr.abort) req.xhr.abort()
373+
374+
destroyRequest()
375+
352376
_emit(new Event('closed'))
353377
}
354378

0 commit comments

Comments
 (0)