Make warning for failed bucket requests less aggressive (#4765)

* make warning for failed bucket requests less aggressive and easier to understand; refactor detailed toasts; catch uncaught promise when client or server is offline; fix toast layout for firefox * update changelog * fix tests
scalableminds · Aug 17, 2020 · 4cf0318 · 4cf0318
1 parent c7a3685
commit 4cf0318
Show file tree

Hide file tree

Showing 8 changed files with 68 additions and 43 deletions.
diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md
@@ -23,6 +23,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
 - The rotation buttons of the 3D-viewport no longer change the zoom. [#4750](https://github.com/scalableminds/webknossos/pull/4750)
 - Improved the performance of applying agglomerate files. [#4706](https://github.com/scalableminds/webknossos/pull/4706)
 - In the Edit/Import Dataset form, the "Sharing" tab was renamed to "Sharing & Permissions". Also, existing permission-related settings were moved to that tab.  [#4683](https://github.com/scalableminds/webknossos/pull/4763)
+- Improved handling and communication of failures during download of data from datasets. [#4765](https://github.com/scalableminds/webknossos/pull/4765)
 
 ### Fixed
 - Speed up NML import in existing tracings for NMLs with many trees (20,000+). [#4742](https://github.com/scalableminds/webknossos/pull/4742)

diff --git a/frontend/javascripts/admin/datastore_health_check.js b/frontend/javascripts/admin/datastore_health_check.js
@@ -25,7 +25,12 @@ const pingDataStoreIfAppropriate = memoizedThrottle(async (requestedUrl: string)
     RestAPI.getDataStoresCached(),
     RestAPI.getTracingStoreCached(),
     RestAPI.isInMaintenance(),
-  ]);
+  ]).catch(() => [null, null, null]);
+  if (datastores == null || tracingstore == null || isInMaintenance == null) {
+    Toast.warning(messages.offline);
+    return;
+  }
+
   const stores = datastores
     .map(datastore => ({ ...datastore, path: "data" }))
     .concat({ ...tracingstore, path: "tracings" });

diff --git a/frontend/javascripts/libs/toast.js b/frontend/javascripts/libs/toast.js
@@ -22,42 +22,45 @@ const Toast = {
         this.success(singleMessage.success);
       }
       if (singleMessage.error != null) {
-        if (errorChainString) {
-          this.renderDetailedErrorMessage(singleMessage.error, errorChainString);
-        } else {
-          this.error(singleMessage.error);
-        }
+        this.error(singleMessage.error, { sticky: true }, errorChainString);
       }
     });
   },
 
-  renderDetailedErrorMessage(
-    errorString: string,
-    errorChain: string,
-    config: ToastConfig = {},
-  ): void {
-    config.sticky = config.sticky != null ? config.sticky : true;
-    this.error(
+  buildContentWithDetails(
+    title: string | React$Element<any>,
+    details: string | React$Element<any> | null,
+  ) {
+    if (!details) {
+      return title;
+    }
+    return (
       <div>
-        {errorString}
+        {title}
         <Collapse
-          className="errorChainCollapse"
+          className="collapsibleToastDetails"
           bordered={false}
           style={{ background: "transparent", marginLeft: -16 }}
         >
           <Panel
             header="Show more information"
             style={{ background: "transparent", border: 0, fontSize: 10 }}
           >
-            {errorChain}
+            {details}
           </Panel>
         </Collapse>
-      </div>,
-      config,
+      </div>
     );
   },
 
-  message(type: ToastStyle, message: string | React$Element<any>, config: ToastConfig): void {
+  message(
+    type: ToastStyle,
+    rawMessage: string | React$Element<any>,
+    config: ToastConfig,
+    details?: string,
+  ): void {
+    const message = this.buildContentWithDetails(rawMessage, details);
+
     const timeout = config.timeout != null ? config.timeout : 6000;
     const key = config.key || (typeof message === "string" ? message : null);
     const { sticky, onClose } = config;
@@ -95,20 +98,24 @@ const Toast = {
     notification[type](toastConfig);
   },
 
-  info(message: string | React$Element<any>, config: ToastConfig = {}): void {
-    return this.message("info", message, config);
+  info(message: string | React$Element<any>, config: ToastConfig = {}, details?: ?string): void {
+    return this.message("info", message, config, details);
   },
 
-  warning(message: string, config: ToastConfig = {}): void {
-    return this.message("warning", message, config);
+  warning(message: string, config: ToastConfig = {}, details?: ?string): void {
+    return this.message("warning", message, config, details);
   },
 
-  success(message: string = "Success :-)", config: ToastConfig = {}): void {
-    return this.message("success", message, config);
+  success(message: string = "Success :-)", config: ToastConfig = {}, details?: ?string): void {
+    return this.message("success", message, config, details);
   },
 
-  error(message: string | React$Element<any> = "Error :-/", config: ToastConfig = {}): void {
-    return this.message("error", message, config);
+  error(
+    message: string | React$Element<any> = "Error :-/",
+    config: ToastConfig = {},
+    details?: ?string,
+  ): void {
+    return this.message("error", message, config, details);
   },
 
   close(key: string) {

diff --git a/frontend/javascripts/messages.js b/frontend/javascripts/messages.js
@@ -46,6 +46,8 @@ export default {
   no: "No",
   unknown_error:
     "An unknown error occurred. Please try again or check the console for more details.",
+  offline:
+    "The communication to the server failed. This can happen when you are offline or when the server is down. Retrying...",
   "datastore.health": _.template(
     "The datastore server at <%- url %> does not seem too be available. Please check back in five minutes.",
   ),

diff --git a/frontend/javascripts/oxalis/model/bucket_data_handling/wkstore_adapter.js b/frontend/javascripts/oxalis/model/bucket_data_handling/wkstore_adapter.js
@@ -23,7 +23,7 @@ import constants, { type Vector3, type Vector4 } from "oxalis/constants";
 const decodeFourBit = createWorker(DecodeFourBitWorker);
 const byteArrayToLz4Base64 = createWorker(ByteArrayToLz4Base64Worker);
 
-export const REQUEST_TIMEOUT = 30000;
+export const REQUEST_TIMEOUT = 60000;
 
 export type SendBucketInfo = {
   position: Vector3,
@@ -166,19 +166,25 @@ export async function requestFromStore(
       return sliceBufferIntoPieces(layerInfo, batch, missingBuckets, new Uint8Array(resultBuffer));
     });
   } catch (errorResponse) {
-    let errorMessage = `Requesting buckets from layer "${layerInfo.name}" failed. `;
-    if (errorResponse.status != null) {
-      errorMessage += `Status code ${errorResponse.status} - "${errorResponse.statusText}".`;
-    } else {
-      errorMessage += errorResponse.message;
-    }
-    const urlAsString = `URL - ${dataUrl}`;
-    console.error(`${errorMessage} ${urlAsString}`);
+    const errorMessage = `Requesting data from layer "${
+      layerInfo.name
+    }" failed. Some rendered areas might remain empty. Retrying...`;
+    let detailedError =
+      errorResponse.status != null
+        ? `Status code ${errorResponse.status} - "${errorResponse.statusText}".`
+        : errorResponse.message;
+
+    detailedError += `(URL: ${dataUrl})`;
+    console.error(`${errorMessage} ${detailedError}`);
     console.error(errorResponse);
-    Toast.renderDetailedErrorMessage(errorMessage, urlAsString, {
-      key: errorMessage,
-      sticky: false,
-    });
+    Toast.warning(
+      errorMessage,
+      {
+        key: errorMessage,
+        sticky: false,
+      },
+      detailedError,
+    );
     return batch.map(_val => null);
   }
 }

diff --git a/frontend/javascripts/oxalis/model/sagas/isosurface_saga.js b/frontend/javascripts/oxalis/model/sagas/isosurface_saga.js
@@ -255,7 +255,7 @@ function* downloadActiveIsosurfaceCell(): Saga<void> {
   const geometry = sceneController.getIsosurfaceGeometry(currentId);
   if (geometry == null) {
     const errorMessages = messages["tracing.not_isosurface_available_to_download"];
-    Toast.renderDetailedErrorMessage(errorMessages[0], errorMessages[1], { sticky: false });
+    Toast.error(errorMessages[0], { sticky: false }, errorMessages[1]);
     return;
   }
   const stl = exportToStl(geometry);

diff --git a/frontend/javascripts/test/model/binary/layers/wkstore_adapter.spec.js b/frontend/javascripts/test/model/binary/layers/wkstore_adapter.spec.js
@@ -45,7 +45,6 @@ const StoreMock = {
 
 mockRequire("libs/request", RequestMock);
 mockRequire("oxalis/store", StoreMock);
-mockRequire("libs/toast", { renderDetailedErrorMessage: _.noop });
 
 const { DataBucket } = mockRequire.reRequire("oxalis/model/bucket_data_handling/bucket");
 const { requestWithFallback, sendToStore } = mockRequire.reRequire(
@@ -128,7 +127,7 @@ function createExpectedOptions(fourBit: boolean = false) {
       { position: [0, 0, 0], zoomStep: 0, cubeSize: 32, fourBit },
       { position: [64, 64, 64], zoomStep: 1, cubeSize: 32, fourBit },
     ],
-    timeout: 30000,
+    timeout: 60000,
     showErrorToast: false,
   };
 }

diff --git a/frontend/stylesheets/antd_overwrites.less b/frontend/stylesheets/antd_overwrites.less
@@ -84,6 +84,11 @@
 
 .ant-notification-notice-message {
   white-space: pre-wrap;
+
+  .ant-notification-notice-message-single-line-auto-margin {
+    // This fixes a too large margin-top for the notification content in Firefox
+    float: left;
+  }
 }
 
 .ant-table-small > .ant-table-content > .ant-table-body > table > .ant-table-tbody > tr > td {