From 9227c303c68fc30cebdb45fddbf808149f7507cc Mon Sep 17 00:00:00 2001 From: Remco Burema Date: Wed, 14 Apr 2021 13:59:21 +0200 Subject: [PATCH 1/3] Fix 'uploaded print-job cache' set before the mesh upload. Since we use that to detect when the mesh is already uploaded, and thus can be reprinted, this could cause problems, since, while we do properly set it to None when an error is returned, if the request never returns to us, or if a reprint is started while the mesh is still uploading, the print-job cache could be set while the mesh wasn't actually there yet. Which could in theory have maybe caused the problems we see. CURA-8004 --- .../src/Cloud/CloudOutputDevice.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py b/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py index 56144a54b3..c4efe6c85a 100644 --- a/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py +++ b/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py @@ -1,4 +1,4 @@ -# Copyright (c) 2020 Ultimaker B.V. +# Copyright (c) 2021 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. from time import time @@ -104,6 +104,7 @@ class CloudOutputDevice(UltimakerNetworkedPrinterOutputDevice): # Reference to the uploaded print job / mesh # We do this to prevent re-uploading the same file multiple times. self._tool_path = None # type: Optional[bytes] + self._pre_upload_print_job = None # type: Optional[CloudPrintJobResponse] self._uploaded_print_job = None # type: Optional[CloudPrintJobResponse] def connect(self) -> None: @@ -130,6 +131,7 @@ class CloudOutputDevice(UltimakerNetworkedPrinterOutputDevice): """Resets the print job that was uploaded to force a new upload, runs whenever the user re-slices.""" self._tool_path = None + self._pre_upload_print_job = None self._uploaded_print_job = None def matchesNetworkKey(self, network_key: str) -> bool: @@ -196,6 +198,7 @@ class CloudOutputDevice(UltimakerNetworkedPrinterOutputDevice): # The mesh didn't change, let's not upload it to the cloud again. # Note that self.writeFinished is called in _onPrintUploadCompleted as well. if self._uploaded_print_job: + Logger.log("i", "Current mesh is already attached to a print-job, immediately request reprint.") self._api.requestPrint(self.key, self._uploaded_print_job.job_id, self._onPrintUploadCompleted, self._onPrintUploadSpecificError) return @@ -227,7 +230,7 @@ class CloudOutputDevice(UltimakerNetworkedPrinterOutputDevice): if not self._tool_path: return self._onUploadError() self._progress.show() - self._uploaded_print_job = job_response # store the last uploaded job to prevent re-upload of the same file + self._pre_upload_print_job = job_response # store the last uploaded job to prevent re-upload of the same file self._api.uploadToolPath(job_response, self._tool_path, self._onPrintJobUploaded, self._progress.update, self._onUploadError) @@ -238,9 +241,11 @@ class CloudOutputDevice(UltimakerNetworkedPrinterOutputDevice): """ self._progress.update(100) - print_job = cast(CloudPrintJobResponse, self._uploaded_print_job) - if not print_job: # It's possible that another print job is requested in the meanwhile, which then fails to upload with an error, which sets self._uploaded_print_job to `None`. - # TODO: Maybe _onUploadError shouldn't set the _uploaded_print_job to None or we need to prevent such asynchronous cases. + print_job = cast(CloudPrintJobResponse, self._pre_upload_print_job) + if not print_job: # It's possible that another print job is requested in the meanwhile, which then fails to upload with an error, which sets self._pre_uploaded_print_job to `None`. + self._pre_upload_print_job = None + self._uploaded_print_job = None + Logger.log("w", "Interference from another job uploaded at roughly the same time, not uploading print!") return # Prevent a crash. self._api.requestPrint(self.key, print_job.job_id, self._onPrintUploadCompleted, self._onPrintUploadSpecificError) @@ -249,6 +254,7 @@ class CloudOutputDevice(UltimakerNetworkedPrinterOutputDevice): :param response: The response from the cloud API. """ + self._uploaded_print_job = self._pre_upload_print_job self._progress.hide() PrintJobUploadSuccessMessage().show() self.writeFinished.emit() @@ -264,6 +270,7 @@ class CloudOutputDevice(UltimakerNetworkedPrinterOutputDevice): PrintJobUploadErrorMessage(I18N_CATALOG.i18nc("@error:send", "Unknown error code when uploading print job: {0}", error_code)).show() self._progress.hide() + self._pre_upload_print_job = None self._uploaded_print_job = None self.writeError.emit() @@ -273,6 +280,7 @@ class CloudOutputDevice(UltimakerNetworkedPrinterOutputDevice): :param message: The message to display. """ self._progress.hide() + self._pre_upload_print_job = None self._uploaded_print_job = None PrintJobUploadErrorMessage(message).show() self.writeError.emit() From 4c5dac0dfdbc908c5b88d9f83a215f3515becce9 Mon Sep 17 00:00:00 2001 From: Remco Burema Date: Wed, 14 Apr 2021 14:30:42 +0200 Subject: [PATCH 2/3] Improve logging, UX for cloud-upload-errors. One of the reasons this bug (see parent of this commit ... or the issue nr if you have internal access) was so vague is that A. the user was insufficiently prompted, and B. no one could find anything in our logs. CURA-8004 --- plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py | 4 ++++ .../src/Messages/PrintJobUploadErrorMessage.py | 3 +-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py b/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py index c4efe6c85a..13a6a54e95 100644 --- a/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py +++ b/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py @@ -269,6 +269,8 @@ class CloudOutputDevice(UltimakerNetworkedPrinterOutputDevice): else: PrintJobUploadErrorMessage(I18N_CATALOG.i18nc("@error:send", "Unknown error code when uploading print job: {0}", error_code)).show() + Logger.log("w", "Upload of print job failed specifically with error code {}".format(error_code)) + self._progress.hide() self._pre_upload_print_job = None self._uploaded_print_job = None @@ -279,6 +281,8 @@ class CloudOutputDevice(UltimakerNetworkedPrinterOutputDevice): Displays the given message if uploading the mesh has failed due to a generic error (i.e. lost connection). :param message: The message to display. """ + Logger.log("w", "Upload error with message {}".format(message)) + self._progress.hide() self._pre_upload_print_job = None self._uploaded_print_job = None diff --git a/plugins/UM3NetworkPrinting/src/Messages/PrintJobUploadErrorMessage.py b/plugins/UM3NetworkPrinting/src/Messages/PrintJobUploadErrorMessage.py index 5145844ea7..9feb4b4970 100644 --- a/plugins/UM3NetworkPrinting/src/Messages/PrintJobUploadErrorMessage.py +++ b/plugins/UM3NetworkPrinting/src/Messages/PrintJobUploadErrorMessage.py @@ -13,6 +13,5 @@ class PrintJobUploadErrorMessage(Message): def __init__(self, message: str = None) -> None: super().__init__( text = message or I18N_CATALOG.i18nc("@info:text", "Could not upload the data to the printer."), - title = I18N_CATALOG.i18nc("@info:title", "Network error"), - lifetime = 10 + title = I18N_CATALOG.i18nc("@info:title", "Network error") ) From 1eb1a943b243d7c4ad6599a5e52f0cd06d14b6d0 Mon Sep 17 00:00:00 2001 From: Remco Burema Date: Thu, 15 Apr 2021 16:53:01 +0200 Subject: [PATCH 3/3] Process supposed to stop when already sending a job. When using the visibility of the progress bar to detect if a job is already being sent, then make actually sure the progress bar is visible the moment the job starts, not at some unspecified time later in a method that might not even trigger if there is already a mesh ... so it's unlikely to even work, since the thing it was intended to prevent _very_ likely has the same mesh anyway. CURA-8004 --- plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py b/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py index 13a6a54e95..1884efec46 100644 --- a/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py +++ b/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py @@ -191,6 +191,7 @@ class CloudOutputDevice(UltimakerNetworkedPrinterOutputDevice): if self._progress.visible: PrintJobUploadBlockedMessage().show() return + self._progress.show() # Indicate we have started sending a job. self.writeStarted.emit(self) @@ -229,7 +230,6 @@ class CloudOutputDevice(UltimakerNetworkedPrinterOutputDevice): """ if not self._tool_path: return self._onUploadError() - self._progress.show() self._pre_upload_print_job = job_response # store the last uploaded job to prevent re-upload of the same file self._api.uploadToolPath(job_response, self._tool_path, self._onPrintJobUploaded, self._progress.update, self._onUploadError)