Skip to content

Commit

Permalink
Fix HTTPLoader (#4403)
Browse files Browse the repository at this point in the history 
* Fix HTTPLoader onerror handler

* Fix HTTPLoader in order to apply response interceptors in all uses cases (errors)

* Complete onloadend callback replacing onerror callback

* FetchLoader: fix reader error handling

* Fix unit tests

* HTTLoader: separate request timing info update function from resourceTiming update function

* HTTPLoader: fix ontimeout handler

* Fix FetchLoader in case aborted request

* Fix FetchLoader in case aborted request
  • Loading branch information
@bbert
bbert authored Mar 21, 2024
1 parent 9c64c5c commit ca3a434
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 138 deletions.
16 changes: 7 additions & 9 deletions src/streaming/net/FetchLoader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ function FetchLoader() {
httpResponse.url = response.url;

if (!response.ok) {
httpRequest.customData.onerror();
httpRequest.customData.onloadend();
}

const responseHeaders = {};
Expand Down Expand Up @@ -187,7 +187,6 @@ function FetchLoader() {

httpResponse.data = receivedData.buffer;
}
httpRequest.customData.onload();
httpRequest.customData.onloadend();
}

Expand Down Expand Up @@ -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();
}
});
});
Expand Down Expand Up @@ -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();
}
});
}
Expand Down
159 changes: 75 additions & 84 deletions src/streaming/net/HTTPLoader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
};

/**
Expand Down Expand Up @@ -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);
}
Expand All @@ -248,73 +267,44 @@ 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) {
return new Promise((resolve) => {
_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;
Expand All @@ -325,7 +315,7 @@ function HTTPLoader(cfg) {
});
});
}

/**
* Retriggers the request in case we did not exceed the number of retry attempts
* @private
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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));

Expand Down Expand Up @@ -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();
}
Expand Down
4 changes: 1 addition & 3 deletions src/streaming/net/XHRLoader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading

0 comments on commit ca3a434

Please sign in to comment.