From 0ca5e10b189f94d2acfacfe246cf730cdeffd43b Mon Sep 17 00:00:00 2001 From: Omar Khashoggi Date: Fri, 21 Aug 2020 16:52:57 +0300 Subject: [PATCH 1/7] handle network and http errors in the polling client --- .../static/celery_progress/celery_progress.js | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/celery_progress/static/celery_progress/celery_progress.js b/celery_progress/static/celery_progress/celery_progress.js index 85dbb44..6623d66 100644 --- a/celery_progress/static/celery_progress/celery_progress.js +++ b/celery_progress/static/celery_progress/celery_progress.js @@ -22,7 +22,7 @@ var CeleryProgressBar = (function () { progressBarMessageElement.innerHTML = progress.current + ' of ' + progress.total + ' processed. ' + description; } - function updateProgress (progressUrl, options) { + async function updateProgress (progressUrl, options) { options = options || {}; var progressBarId = options.progressBarId || 'progress-bar'; var progressBarMessage = options.progressBarMessageId || 'progress-bar-message'; @@ -31,13 +31,22 @@ var CeleryProgressBar = (function () { var onProgress = options.onProgress || onProgressDefault; var onSuccess = options.onSuccess || onSuccessDefault; var onError = options.onError || onErrorDefault; + var onNetworkError = options.onNetworkError || onErrorDefault; + var onHttpError = options.onHttpError || onErrorDefault; var pollInterval = options.pollInterval || 500; var resultElementId = options.resultElementId || 'celery-result'; var resultElement = options.resultElement || document.getElementById(resultElementId); var onResult = options.onResult || onResultDefault; - fetch(progressUrl).then(function(response) { + let response; + try { + response = await fetch(progressUrl); + } catch (networkError) { + onNetworkError(progressBarElement, progressBarMessageElement, "Network Error"); + } + + if (response.status === 200) { response.json().then(function(data) { if (data.progress) { onProgress(progressBarElement, progressBarMessageElement, data.progress); @@ -55,12 +64,16 @@ var CeleryProgressBar = (function () { } } }); - }); + } else { + onHttpError(progressBarElement, progressBarMessageElement, "HTTP Code " + response.status); + } } return { onSuccessDefault: onSuccessDefault, onResultDefault: onResultDefault, onErrorDefault: onErrorDefault, + onNetworkError: onErrorDefault, + onHttpError: onErrorDefault, onProgressDefault: onProgressDefault, updateProgress: updateProgress, initProgressBar: updateProgress, // just for api cleanliness From 530e9926e6807444d76d36d9600118b35aab6672 Mon Sep 17 00:00:00 2001 From: Omar Khashoggi Date: Fri, 21 Aug 2020 21:20:20 +0300 Subject: [PATCH 2/7] handle parsing error and restructure error handlers - there's one default error handler - other errors (task, network, http, data) can be customized - pass the Response when handling http errors --- .../static/celery_progress/celery_progress.js | 53 +++++++++++-------- 1 file changed, 32 insertions(+), 21 deletions(-) diff --git a/celery_progress/static/celery_progress/celery_progress.js b/celery_progress/static/celery_progress/celery_progress.js index 6623d66..1e22384 100644 --- a/celery_progress/static/celery_progress/celery_progress.js +++ b/celery_progress/static/celery_progress/celery_progress.js @@ -10,7 +10,11 @@ var CeleryProgressBar = (function () { } } - function onErrorDefault(progressBarElement, progressBarMessageElement, excMessage) { + /** + * Default handler for all errors. + * @param data - A Response object for HTTP errors, undefined for other errors + */ + function onErrorDefault(progressBarElement, progressBarMessageElement, excMessage, data) { progressBarElement.style.backgroundColor = '#dc4f63'; progressBarMessageElement.innerHTML = "Uh-Oh, something went wrong! " + excMessage; } @@ -31,8 +35,10 @@ var CeleryProgressBar = (function () { var onProgress = options.onProgress || onProgressDefault; var onSuccess = options.onSuccess || onSuccessDefault; var onError = options.onError || onErrorDefault; - var onNetworkError = options.onNetworkError || onErrorDefault; - var onHttpError = options.onHttpError || onErrorDefault; + var onTaskError = options.onTaskError || onError; + var onNetworkError = options.onNetworkError || onError; + var onHttpError = options.onHttpError || onError; + var onDataError = options.onDataError || onError; var pollInterval = options.pollInterval || 500; var resultElementId = options.resultElementId || 'celery-result'; var resultElement = options.resultElement || document.getElementById(resultElementId); @@ -44,36 +50,41 @@ var CeleryProgressBar = (function () { response = await fetch(progressUrl); } catch (networkError) { onNetworkError(progressBarElement, progressBarMessageElement, "Network Error"); + throw networkError; } if (response.status === 200) { - response.json().then(function(data) { - if (data.progress) { - onProgress(progressBarElement, progressBarMessageElement, data.progress); - } - if (!data.complete) { - setTimeout(updateProgress, pollInterval, progressUrl, options); + let data; + try { + data = await response.json(); + } catch (parsingError) { + onDataError(progressBarElement, progressBarMessageElement, "Parsing Error") + throw parsingError; + } + + if (data.progress) { + onProgress(progressBarElement, progressBarMessageElement, data.progress); + } + if (!data.complete) { + setTimeout(updateProgress, pollInterval, progressUrl, options); + } else { + if (data.success) { + onSuccess(progressBarElement, progressBarMessageElement, data.result); } else { - if (data.success) { - onSuccess(progressBarElement, progressBarMessageElement, data.result); - } else { - onError(progressBarElement, progressBarMessageElement, data.result); - } - if (data.result) { - onResult(resultElement, data.result); - } + onTaskError(progressBarElement, progressBarMessageElement, data.result); + } + if (data.result) { + onResult(resultElement, data.result); } - }); + } } else { - onHttpError(progressBarElement, progressBarMessageElement, "HTTP Code " + response.status); + onHttpError(progressBarElement, progressBarMessageElement, "HTTP Code " + response.status, response); } } return { onSuccessDefault: onSuccessDefault, onResultDefault: onResultDefault, onErrorDefault: onErrorDefault, - onNetworkError: onErrorDefault, - onHttpError: onErrorDefault, onProgressDefault: onProgressDefault, updateProgress: updateProgress, initProgressBar: updateProgress, // just for api cleanliness From 1163e2507526615ffff6daf472b9774118a3c1c6 Mon Sep 17 00:00:00 2001 From: Omar Khashoggi Date: Fri, 21 Aug 2020 21:23:45 +0300 Subject: [PATCH 3/7] show error on invalid JSON response and stop polling --- celery_progress/static/celery_progress/celery_progress.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/celery_progress/static/celery_progress/celery_progress.js b/celery_progress/static/celery_progress/celery_progress.js index 1e22384..2277544 100644 --- a/celery_progress/static/celery_progress/celery_progress.js +++ b/celery_progress/static/celery_progress/celery_progress.js @@ -65,13 +65,15 @@ var CeleryProgressBar = (function () { if (data.progress) { onProgress(progressBarElement, progressBarMessageElement, data.progress); } - if (!data.complete) { + if (data.complete === false) { setTimeout(updateProgress, pollInterval, progressUrl, options); } else { - if (data.success) { + if (data.success === true) { onSuccess(progressBarElement, progressBarMessageElement, data.result); - } else { + } else if (data.success === false) { onTaskError(progressBarElement, progressBarMessageElement, data.result); + } else { + onDataError(progressBarElement, progressBarMessageElement, "Data Error"); } if (data.result) { onResult(resultElement, data.result); From ab1d3ec944f7a0a8a14058fc18775007b5f79e51 Mon Sep 17 00:00:00 2001 From: Omar Khashoggi Date: Fri, 21 Aug 2020 21:31:11 +0300 Subject: [PATCH 4/7] resturecture error handling in websocket still, only a task error is recognized by websocket currently --- celery_progress/static/celery_progress/websockets.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/celery_progress/static/celery_progress/websockets.js b/celery_progress/static/celery_progress/websockets.js index e136b03..7f98d82 100644 --- a/celery_progress/static/celery_progress/websockets.js +++ b/celery_progress/static/celery_progress/websockets.js @@ -7,8 +7,8 @@ var CeleryWebSocketProgressBar = (function () { CeleryProgressBar.onResultDefault(resultElement, result); } - function onErrorDefault(progressBarElement, progressBarMessageElement, excMessage) { - CeleryProgressBar.onErrorDefault(progressBarElement, progressBarMessageElement, excMessage); + function onErrorDefault(progressBarElement, progressBarMessageElement, excMessage, data) { + CeleryProgressBar.onErrorDefault(progressBarElement, progressBarMessageElement, excMessage, data); } function onProgressDefault(progressBarElement, progressBarMessageElement, progress) { @@ -24,6 +24,7 @@ var CeleryWebSocketProgressBar = (function () { var onProgress = options.onProgress || onProgressDefault; var onSuccess = options.onSuccess || onSuccessDefault; var onError = options.onError || onErrorDefault; + var onTaskError = options.onTaskError || onError; var resultElementId = options.resultElementId || 'celery-result'; var resultElement = options.resultElement || document.getElementById(resultElementId); var onResult = options.onResult || onResultDefault; @@ -45,7 +46,7 @@ var CeleryWebSocketProgressBar = (function () { if (data.success) { onSuccess(progressBarElement, progressBarMessageElement, data.result); } else { - onError(progressBarElement, progressBarMessageElement, data.result); + onTaskError(progressBarElement, progressBarMessageElement, data.result); } if (data.result) { onResult(resultElement, data.result); From b019752eeeeafbc0f8dd5b0667375e5b56444232 Mon Sep 17 00:00:00 2001 From: Omar Khashoggi Date: Fri, 21 Aug 2020 21:31:56 +0300 Subject: [PATCH 5/7] document error handling customization in README --- README.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index e01b8f9..9d1dcdd 100644 --- a/README.md +++ b/README.md @@ -165,7 +165,11 @@ The `initProgressBar` function takes an optional object of options. The followin | resultElement | Override the *element* used for the result. If specified, resultElementId will be ignored. | document.getElementById(resultElementId) | | onProgress | function to call when progress is updated | CeleryProgressBar.onProgressDefault | | onSuccess | function to call when progress successfully completes | CeleryProgressBar.onSuccessDefault | -| onError | function to call when progress completes with an error | CeleryProgressBar.onErrorDefault | +| onError | function to call when on a known error with no specified handler | CeleryProgressBar.onErrorDefault | +| onTaskError | function to call when progress completes with an error | onError | +| onNetworkError | function to call on a network error (ignored by WebSocket) | onError | +| onHttpError | function to call on a non-200 response (ignored by WebSocket) | onError | +| onDataError | function to call on a 200 response that's not JSON or has invalid schema due to a programming error | onError (ignored by WebSocket) | | onResult | function to call when returned non empty result | CeleryProgressBar.onResultDefault | From cf44f9eac849e6e84cbb2a2d101e8708c56eb11d Mon Sep 17 00:00:00 2001 From: Omar Khashoggi Date: Sun, 23 Aug 2020 13:32:53 +0300 Subject: [PATCH 6/7] use hasOwnProperty to check data.result exists --- celery_progress/static/celery_progress/celery_progress.js | 2 +- celery_progress/static/celery_progress/websockets.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/celery_progress/static/celery_progress/celery_progress.js b/celery_progress/static/celery_progress/celery_progress.js index 2277544..e0a58d9 100644 --- a/celery_progress/static/celery_progress/celery_progress.js +++ b/celery_progress/static/celery_progress/celery_progress.js @@ -75,7 +75,7 @@ var CeleryProgressBar = (function () { } else { onDataError(progressBarElement, progressBarMessageElement, "Data Error"); } - if (data.result) { + if (data.hasOwnProperty('result')) { onResult(resultElement, data.result); } } diff --git a/celery_progress/static/celery_progress/websockets.js b/celery_progress/static/celery_progress/websockets.js index 7f98d82..6dfb78f 100644 --- a/celery_progress/static/celery_progress/websockets.js +++ b/celery_progress/static/celery_progress/websockets.js @@ -48,7 +48,7 @@ var CeleryWebSocketProgressBar = (function () { } else { onTaskError(progressBarElement, progressBarMessageElement, data.result); } - if (data.result) { + if (data.hasOwnProperty('result')) { onResult(resultElement, data.result); } ProgressSocket.close(); From 1b141a1426c421bf488747630ccd298e03a204ef Mon Sep 17 00:00:00 2001 From: Omar Khashoggi Date: Sun, 23 Aug 2020 13:47:35 +0300 Subject: [PATCH 7/7] fix README documentation of error handlers --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 9d1dcdd..b8f46e0 100644 --- a/README.md +++ b/README.md @@ -165,11 +165,11 @@ The `initProgressBar` function takes an optional object of options. The followin | resultElement | Override the *element* used for the result. If specified, resultElementId will be ignored. | document.getElementById(resultElementId) | | onProgress | function to call when progress is updated | CeleryProgressBar.onProgressDefault | | onSuccess | function to call when progress successfully completes | CeleryProgressBar.onSuccessDefault | -| onError | function to call when on a known error with no specified handler | CeleryProgressBar.onErrorDefault | +| onError | function to call on a known error with no specified handler | CeleryProgressBar.onErrorDefault | | onTaskError | function to call when progress completes with an error | onError | | onNetworkError | function to call on a network error (ignored by WebSocket) | onError | | onHttpError | function to call on a non-200 response (ignored by WebSocket) | onError | -| onDataError | function to call on a 200 response that's not JSON or has invalid schema due to a programming error | onError (ignored by WebSocket) | +| onDataError | function to call on a 200 response that's not JSON or has invalid schema due to a programming error (ignored by WebSocket) | onError | | onResult | function to call when returned non empty result | CeleryProgressBar.onResultDefault |