diff --git a/src/streaming/net/FetchLoader.js b/src/streaming/net/FetchLoader.js index 2b37926f08..4b03aab6db 100644 --- a/src/streaming/net/FetchLoader.js +++ b/src/streaming/net/FetchLoader.js @@ -111,7 +111,7 @@ function FetchLoader() { httpResponse.url = response.url; if (!response.ok) { - httpRequest.customData.onerror(); + httpRequest.customData.onloadend(); } const responseHeaders = {}; @@ -187,7 +187,6 @@ function FetchLoader() { httpResponse.data = receivedData.buffer; } - httpRequest.customData.onload(); httpRequest.customData.onloadend(); } @@ -270,9 +269,9 @@ function FetchLoader() { _read(httpRequest, httpResponse, _processResult); }) - .catch(function (e) { - if (httpRequest.customData.onerror) { - httpRequest.customData.onerror(e); + .catch(function () { + if (httpRequest.customData.onloadend) { + httpRequest.customData.onloadend(); } }); }); @@ -342,10 +341,9 @@ function FetchLoader() { function _read(httpRequest, httpResponse, processResult) { httpRequest.customData.reader.read() .then(processResult) - .catch(function (e) { - if (httpRequest.customData.onerror && httpResponse.status === 200) { - // Error, but response code is 200, trigger error - httpRequest.customData.onerror(e); + .catch(function () { + if (httpRequest.customData.onloadend) { + httpRequest.customData.onloadend(); } }); } diff --git a/src/streaming/net/HTTPLoader.js b/src/streaming/net/HTTPLoader.js index 8050942188..8b79af2f7e 100644 --- a/src/streaming/net/HTTPLoader.js +++ b/src/streaming/net/HTTPLoader.js @@ -133,19 +133,12 @@ function HTTPLoader(cfg) { * @private */ function _internalLoad(config, remainingAttempts) { + /** - * Fired when a request has completed, whether successfully (after load) or unsuccessfully (after abort or error). + * Fired when a request has completed, whether successfully (after load) or unsuccessfully (after abort, timeout or error). */ const _onloadend = function () { - // Remove the request from our list of requests - if (httpRequests.indexOf(httpRequest) !== -1) { - httpRequests.splice(httpRequests.indexOf(httpRequest), 1); - } - - if (progressTimeout) { - clearTimeout(progressTimeout); - progressTimeout = null; - } + _onRequestEnd(); }; /** @@ -208,38 +201,64 @@ function HTTPLoader(cfg) { } }; - const _oncomplete = function () { - // Update request timing info - requestObject.startDate = requestStartTime; - requestObject.endDate = new Date(); - requestObject.firstByteDate = requestObject.firstByteDate || requestStartTime; - httpResponse.resourceTiming.responseEnd = requestObject.endDate.getTime(); - - commonAccessTokenController.processResponseHeaders(httpResponse) - - // If enabled the ResourceTimingApi we add the corresponding information to the request object. - // These values are more accurate and can be used by the ThroughputController later - _addResourceTimingValues(httpRequest, httpResponse); - } + /** + * Fired when a request has been aborted, for example because the program called XMLHttpRequest.abort(). + */ + const _onabort = function () { + _onRequestEnd(true) + }; /** - * Fired when an XMLHttpRequest transaction completes. - * This includes status codes such as 404. We handle errors in the _onError function. + * Fired when progress is terminated due to preset time expiring. + * @param event */ - const _onload = function () { - _oncomplete(); + const _ontimeout = function (event) { + let timeoutMessage; + // We know how much we already downloaded by looking at the timeout event + if (event.lengthComputable) { + let percentageComplete = (event.loaded / event.total) * 100; + timeoutMessage = 'Request timeout: loaded: ' + event.loaded + ', out of: ' + event.total + ' : ' + percentageComplete.toFixed(3) + '% Completed'; + } else { + timeoutMessage = 'Request timeout: non-computable download size'; + } + logger.warn(timeoutMessage); + }; + + const _onRequestEnd = function (aborted = false) { + // Remove the request from our list of requests + if (httpRequests.indexOf(httpRequest) !== -1) { + httpRequests.splice(httpRequests.indexOf(httpRequest), 1); + } + + if (progressTimeout) { + clearTimeout(progressTimeout); + progressTimeout = null; + } + + commonAccessTokenController.processResponseHeaders(httpResponse); + + _updateRequestTimingInfo(); + _updateResourceTimingInfo(); _applyResponseInterceptors(httpResponse).then((_httpResponse) => { httpResponse = _httpResponse; _addHttpRequestMetric(httpRequest, httpResponse, traces); + // Ignore aborted requests + if (aborted) { + if (config.abort) { + config.abort(requestObject); + } + return; + } + if (requestObject.type === HTTPRequest.MPD_TYPE) { dashMetrics.addManifestUpdate(requestObject); eventBus.trigger(Events.MANIFEST_LOADING_FINISHED, { requestObject }); } - if (httpResponse.status >= 200 && httpResponse.status <= 299) { + if (httpResponse.status >= 200 && httpResponse.status <= 299 && httpResponse.data) { if (config.success) { config.success(httpResponse.data, httpResponse.statusText, httpResponse.url); } @@ -248,63 +267,36 @@ function HTTPLoader(cfg) { config.complete(requestObject, httpResponse.statusText); } } else { - _onerror(); + // If we get a 404 to a media segment we should check the client clock again and perform a UTC sync in the background. + try { + if (httpResponse.status === 404 && settings.get().streaming.utcSynchronization.enableBackgroundSyncAfterSegmentDownloadError && requestObject.type === HTTPRequest.MEDIA_SEGMENT_TYPE) { + // Only trigger a sync if the loading failed for the first time + const initialNumberOfAttempts = mediaPlayerModel.getRetryAttemptsForType(HTTPRequest.MEDIA_SEGMENT_TYPE); + if (initialNumberOfAttempts === remainingAttempts) { + eventBus.trigger(Events.ATTEMPT_BACKGROUND_SYNC); + } + } + } catch (e) { + } + + _retriggerRequest(); } }); - }; - /** - * Fired when a request has been aborted, for example because the program called XMLHttpRequest.abort(). - */ - const _onabort = function () { - _oncomplete(); - _addHttpRequestMetric(httpRequest, httpResponse, traces); - if (progressTimeout) { - clearTimeout(progressTimeout); - progressTimeout = null; - } - if (config.abort) { - config.abort(requestObject); - } }; - /** - * Fired when progress is terminated due to preset time expiring. - * @param event - */ - const _ontimeout = function (event) { - _oncomplete(); - - let timeoutMessage; - // We know how much we already downloaded by looking at the timeout event - if (event.lengthComputable) { - let percentageComplete = (event.loaded / event.total) * 100; - timeoutMessage = 'Request timeout: loaded: ' + event.loaded + ', out of: ' + event.total + ' : ' + percentageComplete.toFixed(3) + '% Completed'; - } else { - timeoutMessage = 'Request timeout: non-computable download size'; - } - logger.warn(timeoutMessage); - _addHttpRequestMetric(httpRequest, httpResponse, traces); - _retriggerRequest(); - }; + const _updateRequestTimingInfo = function() { + requestObject.startDate = requestStartTime; + requestObject.endDate = new Date(); + requestObject.firstByteDate = requestObject.firstByteDate || requestStartTime; + } - /** - * Fired when the request encountered an error. - */ - const _onerror = function () { - // If we get a 404 to a media segment we should check the client clock again and perform a UTC sync in the background. - try { - if (httpResponse.status === 404 && settings.get().streaming.utcSynchronization.enableBackgroundSyncAfterSegmentDownloadError && requestObject.type === HTTPRequest.MEDIA_SEGMENT_TYPE) { - // Only trigger a sync if the loading failed for the first time - const initialNumberOfAttempts = mediaPlayerModel.getRetryAttemptsForType(HTTPRequest.MEDIA_SEGMENT_TYPE); - if (initialNumberOfAttempts === remainingAttempts) { - eventBus.trigger(Events.ATTEMPT_BACKGROUND_SYNC); - } - } - } catch (e) { - } + const _updateResourceTimingInfo = function() { + httpResponse.resourceTiming.responseEnd = Date.now(); - _retriggerRequest(); + // If enabled the ResourceTimingApi we add the corresponding information to the request object. + // These values are more accurate and can be used by the ThroughputController later + _addResourceTimingValues(httpRequest, httpResponse); } const _loadRequest = function (loader, httpRequest, httpResponse) { @@ -312,9 +304,7 @@ function HTTPLoader(cfg) { _applyRequestInterceptors(httpRequest).then((_httpRequest) => { httpRequest = _httpRequest; - httpRequest.customData.onload = _onload; httpRequest.customData.onloadend = _onloadend; - httpRequest.customData.onerror = _onloadend; httpRequest.customData.onprogress = _onprogress; httpRequest.customData.onabort = _onabort; httpRequest.customData.ontimeout = _ontimeout; @@ -325,7 +315,7 @@ function HTTPLoader(cfg) { }); }); } - + /** * Retriggers the request in case we did not exceed the number of retry attempts * @private @@ -413,7 +403,8 @@ function HTTPLoader(cfg) { resourceTiming: { startTime: Date.now(), encodedBodySize: 0 - } + }, + status: 0 }; // Adds the ability to delay single fragment loading time to control buffer. @@ -441,7 +432,7 @@ function HTTPLoader(cfg) { httpRequests.push(delayedRequest.httpRequest); _loadRequest(loader, delayedRequest.httpRequest, delayedRequest.httpResponse); } catch (e) { - delayedRequest.httpRequest.onerror(); + delayedRequest.httpRequest.onloadend(); } }, (requestObject.delayLoadingTime - now)); @@ -656,7 +647,7 @@ function HTTPLoader(cfg) { // abort will trigger onloadend which we don't want // when deliberately aborting inflight requests - // set them to undefined so they are not called - reqData.onloadend = reqData.onerror = reqData.onprogress = undefined; + reqData.onloadend = reqData.onprogress = undefined; if (reqData.abort) { reqData.abort(); } diff --git a/src/streaming/net/XHRLoader.js b/src/streaming/net/XHRLoader.js index 2f0928ecab..77bfd2220c 100644 --- a/src/streaming/net/XHRLoader.js +++ b/src/streaming/net/XHRLoader.js @@ -66,16 +66,14 @@ function XHRLoader() { xhr.withCredentials = httpRequest.credentials === 'include'; xhr.timeout = httpRequest.timeout; - xhr.onload = function(e) { + xhr.onload = function() { httpResponse.url = this.responseURL; httpResponse.status = this.status; httpResponse.statusText = this.statusText; httpResponse.headers = Utils.parseHttpHeaders(this.getAllResponseHeaders()); httpResponse.data = this.response; - httpRequest.customData.onload(e); } xhr.onloadend = httpRequest.customData.onloadend; - xhr.onerror = httpRequest.customData.onerror; xhr.onprogress = httpRequest.customData.onprogress; xhr.onabort = httpRequest.customData.onabort; xhr.ontimeout = httpRequest.customData.ontimeout; diff --git a/test/unit/streaming.net.XHRLoader.js b/test/unit/streaming.net.XHRLoader.js index 05063f0c81..caffb428b5 100644 --- a/test/unit/streaming.net.XHRLoader.js +++ b/test/unit/streaming.net.XHRLoader.js @@ -25,107 +25,80 @@ describe('XHRLoader', function () { afterEach(function () { }); - it('should call success and complete callback when load is called successfully', () => { + it('should call onloadend callback when load is called successfully', () => { const self = this.ctx; - const callbackSucceeded = sinon.spy(); - const callbackCompleted = sinon.spy(); - const callbackError = sinon.spy(); + const callbackLoadend = sinon.spy(); const callbackAbort = sinon.spy(); xhrLoader = XHRLoader(context).create({}); const request = { request: {}, customData: { - onload: callbackSucceeded, - onloadend: callbackCompleted, - onerror: callbackError, + onloadend: callbackLoadend, onabort: callbackAbort } }; xhrLoader.load(request, {}); expect(self.requests.length).to.equal(1); self.requests[0].respond(200); - sinon.assert.notCalled(callbackError); - sinon.assert.calledOnce(callbackSucceeded); - sinon.assert.calledOnce(callbackCompleted); + sinon.assert.calledOnce(callbackLoadend); sinon.assert.notCalled(callbackAbort); - expect(callbackSucceeded.calledBefore(callbackCompleted)).to.be.true; // jshint ignore:line }); - it('should call onload and complete callback when load is called and there is an error response', () => { + it('should call onloadend callback when load is called and there is an error response', () => { const self = this.ctx; - const callbackSucceeded = sinon.spy(); - const callbackCompleted = sinon.spy(); - const callbackError = sinon.spy(); + const callbackLoadend = sinon.spy(); const callbackAbort = sinon.spy(); xhrLoader = XHRLoader(context).create({}); const request = { customData: { - onload: callbackSucceeded, - onloadend: callbackCompleted, - onerror: callbackError, + onloadend: callbackLoadend, onabort: callbackAbort } }; xhrLoader.load(request, {}); expect(self.requests.length).to.equal(1); self.requests[0].respond(404); - sinon.assert.notCalled(callbackError); - sinon.assert.calledOnce(callbackCompleted); - sinon.assert.calledOnce(callbackSucceeded); + sinon.assert.calledOnce(callbackLoadend); sinon.assert.notCalled(callbackAbort); - expect(callbackSucceeded.calledBefore(callbackCompleted)).to.be.true; // jshint ignore:line }); it('should call onabort callback when abort is called', () => { - const callbackSucceeded = sinon.spy(); - const callbackCompleted = sinon.spy(); - const callbackError = sinon.spy(); + const callbackLoadend = sinon.spy(); const callbackAbort = sinon.spy(); xhrLoader = XHRLoader(context).create({}); const request = { customData: { - onload: callbackSucceeded, - onloadend: callbackCompleted, - onerror: callbackError, + onloadend: callbackLoadend, onabort: callbackAbort } }; xhrLoader.load(request, {}); xhrLoader.abort(); - sinon.assert.notCalled(callbackError); - sinon.assert.notCalled(callbackSucceeded); // abort triggers both onloadend and onabort - sinon.assert.notCalled(callbackCompleted); + sinon.assert.notCalled(callbackLoadend); sinon.assert.calledOnce(callbackAbort); }); - it('should call onerror and onloadend when load is called and there is a network error', () => { + it('should call onloadend when load is called and there is a network error', () => { const self = this.ctx; - const callbackSucceeded = sinon.spy(); - const callbackCompleted = sinon.spy(); - const callbackError = sinon.spy(); + const callbackLoadend = sinon.spy(); const callbackAbort = sinon.spy(); xhrLoader = XHRLoader(context).create({}); const request = { customData: { - onload: callbackSucceeded, - onloadend: callbackCompleted, - onerror: callbackError, + onloadend: callbackLoadend, onabort: callbackAbort } }; xhrLoader.load(request, {}); expect(self.requests.length).to.equal(1); self.requests[0].error(); - sinon.assert.calledOnce(callbackError); - sinon.assert.calledOnce(callbackCompleted); - sinon.assert.notCalled(callbackSucceeded); + sinon.assert.calledOnce(callbackLoadend); sinon.assert.notCalled(callbackAbort); - expect(callbackError.calledBefore(callbackCompleted)).to.be.true; // jshint ignore:line }); it('should set timeout on the sending XHR request', () => {