Bug fix: make dismiss during export not fire success toast (#3882)

* Bug fix: make dismiss during export not fire success toast

* Fix export fail test, since this failure errors early now

* Remove throttling from send side

* Move toast.loading out to when first engine command is sent, so it is shown immediately

* Use shared, named constants for toast messages

* Hook up a couple other error toasts to the `pendingExport.toastId`

e2e/playwright/regression-tests.spec.ts

@@ -346,10 +346,7 @@ const sketch001 = startSketchAt([-0, -0])
     // Find the toast.
     // Look out for the toast message
     const exportingToastMessage = page.getByText(`Exporting...`)
-    await expect(exportingToastMessage).toBeVisible()
-
     const errorToastMessage = page.getByText(`Error while exporting`)
-    await expect(errorToastMessage).toBeVisible()
 
     const engineErrorToastMessage = page.getByText(`Nothing to export`)
     await expect(engineErrorToastMessage).toBeVisible()

src/components/ModelingMachineProvider.tsx

@@ -415,20 +415,9 @@ export const ModelingMachineProvider = ({
             selection: { type: 'default_scene' },
           }
 
-          // Artificially delay the export in playwright tests
-          toast
-            .promise(
           exportFromEngine({
             format: format,
-              }),
+          }).catch(reportRejection)
-
-              {
-                loading: 'Starting print...',
-                success: 'Started print successfully',
-                error: 'Error while starting print',
-              }
-            )
-            .catch(reportRejection)
         },
         'Engine export': ({ event }) => {
           if (event.type !== 'Export') return
@@ -482,18 +471,9 @@ export const ModelingMachineProvider = ({
             format.selection = { type: 'default_scene' }
           }
 
-          toast
-            .promise(
           exportFromEngine({
             format: format as Models['OutputFormat_type'],
-              }),
+          }).catch(reportRejection)
-              {
-                loading: 'Exporting...',
-                success: 'Exported successfully',
-                error: 'Error while exporting',
-              }
-            )
-            .catch(reportRejection)
         },
         'Submit to Text-to-CAD API': ({ event }) => {
           if (event.type !== 'Text-to-CAD') return
@@ -591,7 +571,9 @@ export const ModelingMachineProvider = ({
             else if (kclManager.ast.body.length === 0)
               errorMessage += 'due to Empty Scene'
             console.error(errorMessage)
-            toast.error(errorMessage)
+            toast.error(errorMessage, {
+              id: kclManager.engineCommandManager.pendingExport?.toastId,
+            })
             return false
           }
         },

src/lang/std/engineConnection.ts

@@ -16,7 +16,11 @@ import { useModelingContext } from 'hooks/useModelingContext'
 import { exportMake } from 'lib/exportMake'
 import toast from 'react-hot-toast'
 import { SettingsViaQueryString } from 'lib/settings/settingsTypes'
-import { EXECUTE_AST_INTERRUPT_ERROR_MESSAGE } from 'lib/constants'
+import {
+  EXECUTE_AST_INTERRUPT_ERROR_MESSAGE,
+  EXPORT_TOAST_MESSAGES,
+  MAKE_TOAST_MESSAGES,
+} from 'lib/constants'
 import { KclManager } from 'lang/KclSingleton'
 import { reportRejection } from 'lib/trap'
 
@@ -959,7 +963,9 @@ class EngineConnection extends EventTarget {
               ) {
                 // Reject the promise with the error.
                 this.engineCommandManager.pendingExport.reject(errorsString)
-                toast.error(errorsString)
+                toast.error(errorsString, {
+                  id: this.engineCommandManager.pendingExport.toastId,
+                })
                 this.engineCommandManager.pendingExport = undefined
               }
             } else {
@@ -1327,8 +1333,13 @@ export class EngineCommandManager extends EventTarget {
   defaultPlanes: DefaultPlanes | null = null
   commandLogs: CommandLog[] = []
   pendingExport?: {
+    /** The id of the shared loading/success/error toast for export */
+    toastId: string
+    /** An on-success callback */
     resolve: (a: null) => void
+    /** An on-error callback */
     reject: (reason: string) => void
+    /** The engine command uuid */
     commandId: string
   }
   settings: SettingsViaQueryString
@@ -1590,7 +1601,7 @@ export class EngineCommandManager extends EventTarget {
           // because in all other cases we send JSON strings. But in the case of
           // export we send a binary blob.
           // Pass this to our export function.
-          if (this.exportIntent === null) {
+          if (this.exportIntent === null || this.pendingExport === undefined) {
             toast.error(
               'Export intent was not set, but export data was received'
             )
@@ -1602,19 +1613,22 @@ export class EngineCommandManager extends EventTarget {
 
           switch (this.exportIntent) {
             case ExportIntent.Save: {
-              exportSave(event.data).then(() => {
+              exportSave(event.data, this.pendingExport.toastId).then(() => {
                 this.pendingExport?.resolve(null)
               }, this.pendingExport?.reject)
               break
             }
             case ExportIntent.Make: {
-              exportMake(event.data).then((result) => {
+              exportMake(event.data, this.pendingExport.toastId).then(
+                (result) => {
                   if (result) {
                     this.pendingExport?.resolve(null)
                   } else {
                     this.pendingExport?.reject('Failed to make export')
                   }
-              }, this.pendingExport?.reject)
+                },
+                this.pendingExport?.reject
+              )
               break
             }
           }
@@ -1929,7 +1943,20 @@ export class EngineCommandManager extends EventTarget {
       return Promise.resolve(null)
     } else if (cmd.type === 'export') {
       const promise = new Promise<null>((resolve, reject) => {
+        if (this.exportIntent === null) {
+          if (this.exportIntent === null) {
+            toast.error('Export intent was not set, but export is being sent')
+            console.error('Export intent was not set, but export is being sent')
+            return
+          }
+        }
+        const toastId = toast.loading(
+          this.exportIntent === ExportIntent.Save
+            ? EXPORT_TOAST_MESSAGES.START
+            : MAKE_TOAST_MESSAGES.START
+        )
         this.pendingExport = {
+          toastId,
           resolve: (passThrough) => {
             this.addCommandLog({
               type: 'export-done',

src/lib/browserSaveFile.ts

@@ -1,8 +1,16 @@
 /// The method below uses the File System Access API when it's supported and
 // else falls back to the classic approach. In both cases the function saves
 // the file, but in case of where the File System Access API is supported, the
+
+import toast from 'react-hot-toast'
+import { EXPORT_TOAST_MESSAGES } from './constants'
+
 // user will get a file save dialog where they can choose where the file should be saved.
-export const browserSaveFile = async (blob: Blob, suggestedName: string) => {
+export const browserSaveFile = async (
+  blob: Blob,
+  suggestedName: string,
+  toastId: string
+) => {
   // Feature detection. The API needs to be supported
   // and the app not run in an iframe.
   const supportsFileSystemAccess =
@@ -29,11 +37,15 @@ export const browserSaveFile = async (blob: Blob, suggestedName: string) => {
       const writable = await handle.createWritable()
       await writable.write(blob)
       await writable.close()
+      toast.success(EXPORT_TOAST_MESSAGES.SUCCESS, { id: toastId })
       return
     } catch (err: any) {
       // Fail silently if the user has simply canceled the dialog.
-      if (err.name !== 'AbortError') {
+      if (err.name === 'AbortError') {
+        toast.dismiss(toastId)
+      } else {
         console.error(err.name, err.message)
+        toast.error(EXPORT_TOAST_MESSAGES.FAILED, { id: toastId })
       }
       return
     }
@@ -54,4 +66,5 @@ export const browserSaveFile = async (blob: Blob, suggestedName: string) => {
     URL.revokeObjectURL(blobURL)
     a.remove()
   }, 1000)
+  toast.success(EXPORT_TOAST_MESSAGES.SUCCESS, { id: toastId })
 }

src/lib/constants.ts

@@ -77,3 +77,21 @@ export const PLAYWRIGHT_KEY = 'playwright'
  * allows us to match if the execution of executeAst was interrupted */
 export const EXECUTE_AST_INTERRUPT_ERROR_MESSAGE =
   'Force interrupt, executionIsStale, new AST requested'
+
+/** The messages that appear for exporting toasts */
+export const EXPORT_TOAST_MESSAGES = {
+  START: 'Exporting...',
+  SUCCESS: 'Exported successfully',
+  FAILED: 'Export failed',
+}
+
+/** The messages that appear for "make" command toasts */
+export const MAKE_TOAST_MESSAGES = {
+  START: 'Starting print...',
+  NO_MACHINES: 'No machines available',
+  NO_MACHINE_API_IP: 'No machine api ip available',
+  NO_CURRENT_MACHINE: 'No current machine available',
+  NO_MACHINE_ID: 'No machine id available',
+  ERROR_STARTING_PRINT: 'Error while starting print',
+  SUCCESS: 'Started print successfully',
+}

src/lib/exportFromEngine.ts

@@ -1,7 +1,6 @@
 import { engineCommandManager } from 'lib/singletons'
 import { type Models } from '@kittycad/lib'
 import { uuidv4 } from 'lib/utils'
-import { IS_PLAYWRIGHT_KEY } from '../../e2e/playwright/storageStates'
 
 // Isolating a function to call the engine to export the current scene.
 // Because it has given us trouble in automated testing environments.
@@ -23,11 +22,5 @@ export async function exportFromEngine({
     cmd_id: uuidv4(),
   })
 
-  // If we are in playwright slow down the export.
-  const inPlaywright = window.localStorage.getItem(IS_PLAYWRIGHT_KEY)
-  if (inPlaywright === 'true') {
-    await new Promise((resolve) => setTimeout(resolve, 2000))
-  }
-
   return exportPromise
 }

src/lib/exportMake.ts

@@ -3,33 +3,37 @@ import { machineManager } from './machineManager'
 import toast from 'react-hot-toast'
 import { components } from './machine-api'
 import ModelingAppFile from './modelingAppFile'
+import { MAKE_TOAST_MESSAGES } from './constants'
 
 // Make files locally from an export call.
-export async function exportMake(data: ArrayBuffer): Promise<Response | null> {
+export async function exportMake(
+  data: ArrayBuffer,
+  toastId: string
+): Promise<Response | null> {
   if (machineManager.machineCount() === 0) {
-    console.error('No machines available')
+    console.error(MAKE_TOAST_MESSAGES.NO_MACHINES)
-    toast.error('No machines available')
+    toast.error(MAKE_TOAST_MESSAGES.NO_MACHINES, { id: toastId })
     return null
   }
 
   const machineApiIp = machineManager.machineApiIp
   if (!machineApiIp) {
-    console.error('No machine api ip available')
+    console.error(MAKE_TOAST_MESSAGES.NO_MACHINE_API_IP)
-    toast.error('No machine api ip available')
+    toast.error(MAKE_TOAST_MESSAGES.NO_MACHINE_API_IP, { id: toastId })
     return null
   }
 
   const currentMachine = machineManager.currentMachine
   if (!currentMachine) {
-    console.error('No current machine available')
+    console.error(MAKE_TOAST_MESSAGES.NO_CURRENT_MACHINE)
-    toast.error('No current machine available')
+    toast.error(MAKE_TOAST_MESSAGES.NO_CURRENT_MACHINE, { id: toastId })
     return null
   }
 
   let machineId = currentMachine?.id
   if (!machineId) {
-    console.error('No machine id available', currentMachine)
+    console.error(MAKE_TOAST_MESSAGES.NO_MACHINE_ID, currentMachine)
-    toast.error('No machine id available')
+    toast.error(MAKE_TOAST_MESSAGES.NO_MACHINE_ID, { id: toastId })
     return null
   }
 
@@ -58,16 +62,22 @@ export async function exportMake(data: ArrayBuffer): Promise<Response | null> {
     console.log('response', response)
 
     if (!response.ok) {
-      console.error('Error exporting', response)
+      console.error(MAKE_TOAST_MESSAGES.ERROR_STARTING_PRINT, response)
       const text = await response.text()
-      toast.error('Error exporting: ' + response.statusText + ' ' + text)
+      toast.error(
+        'Error while starting print: ' + response.statusText + ' ' + text,
+        {
+          id: toastId,
+        }
+      )
       return null
     }
 
+    toast.success(MAKE_TOAST_MESSAGES.SUCCESS, { id: toastId })
     return response
   } catch (error) {
-    console.error('Error exporting', error)
+    console.error(MAKE_TOAST_MESSAGES.ERROR_STARTING_PRINT, error)
-    toast.error('Error exporting')
+    toast.error(MAKE_TOAST_MESSAGES.ERROR_STARTING_PRINT, { id: toastId })
     return null
   }
 }

src/lib/exportSave.ts

@@ -4,8 +4,10 @@ import { browserSaveFile } from './browserSaveFile'
 
 import JSZip from 'jszip'
 import ModelingAppFile from './modelingAppFile'
+import toast from 'react-hot-toast'
+import { EXPORT_TOAST_MESSAGES } from './constants'
 
-const save_ = async (file: ModelingAppFile) => {
+const save_ = async (file: ModelingAppFile, toastId: string) => {
   try {
     if (isDesktop()) {
       const extension = file.name.split('.').pop() || null
@@ -20,6 +22,7 @@ const save_ = async (file: ModelingAppFile) => {
           file.name,
           new Uint8Array(file.contents)
         )
+        toast.success(EXPORT_TOAST_MESSAGES.SUCCESS, { id: toastId })
         return
       }
 
@@ -36,13 +39,17 @@ const save_ = async (file: ModelingAppFile) => {
 
       // The user canceled the save.
       // Return early.
-      if (filePathMeta.canceled) return
+      if (filePathMeta.canceled) {
+        toast.dismiss(toastId)
+        return
+      }
 
       // Write the file.
       await window.electron.writeFile(
         filePathMeta.filePath,
         new Uint8Array(file.contents)
       )
+      toast.success(EXPORT_TOAST_MESSAGES.SUCCESS, { id: toastId })
     } else {
       // Download the file to the user's computer.
       // Now we need to download the files to the user's downloads folder.
@@ -51,16 +58,17 @@ const save_ = async (file: ModelingAppFile) => {
       // Create a new blob.
       const blob = new Blob([new Uint8Array(file.contents)])
       // Save the file.
-      await browserSaveFile(blob, file.name)
+      await browserSaveFile(blob, file.name, toastId)
     }
   } catch (e) {
     // TODO: do something real with the error.
     console.error('export error', e)
+    toast.error(EXPORT_TOAST_MESSAGES.FAILED, { id: toastId })
   }
 }
 
 // Saves files locally from an export call.
-export async function exportSave(data: ArrayBuffer) {
+export async function exportSave(data: ArrayBuffer, toastId: string) {
   // This converts the ArrayBuffer to a Rust equivalent Vec<u8>.
   let uintArray = new Uint8Array(data)
 
@@ -72,9 +80,9 @@ export async function exportSave(data: ArrayBuffer) {
       zip.file(file.name, new Uint8Array(file.contents), { binary: true })
     }
     return zip.generateAsync({ type: 'array' }).then((contents) => {
-      return save_({ name: 'output.zip', contents })
+      return save_({ name: 'output.zip', contents }, toastId)
     })
   } else {
-    return save_(files[0])
+    return save_(files[0], toastId)
   }
 }