refactor: added some type guards (#1833)

Signed-off-by: Pedro Lamas <pedrolamas@gmail.com>

Author: pedrolamas

src/components/widgets/filesystem/FileSystem.vue

@@ -1232,12 +1232,18 @@ export default class FileSystem extends Mixins(StateMixin, FilesMixin, ServicesM
       if (hasFileDataTransferTypeInDataTransfer(event.dataTransfer, 'files')) {
         const files = getFileDataTransferDataFromDataTransfer(event.dataTransfer, 'files')
 
-        for (const file of files.items) {
-          const src = `${files.path}/${file}`
-          const dest = `${this.currentPath}/${file}`
-          SocketActions.serverFilesCopy(src, dest)
+        if (files) {
+          for (const file of files.items) {
+            const src = `${files.path}/${file}`
+            const dest = `${this.currentPath}/${file}`
+            SocketActions.serverFilesCopy(src, dest)
+          }
+
+          return
         }
-      } else if (hasFilesInDataTransfer(event.dataTransfer)) {
+      }
+
+      if (hasFilesInDataTransfer(event.dataTransfer)) {
         const files = await getFilesFromDataTransfer(event.dataTransfer)
 
         if (files) {

src/components/widgets/gcode-preview/GcodePreviewCard.vue

@@ -442,12 +442,15 @@ export default class GcodePreviewCard extends Mixins(StateMixin, FilesMixin, Bro
       hasFileDataTransferTypeInDataTransfer(event.dataTransfer, 'jobs')
     ) {
       const files = getFileDataTransferDataFromDataTransfer(event.dataTransfer, 'jobs')
-      const path = files.path ? `gcodes/${files.path}` : 'gcodes'
 
-      const file: AppFile | undefined = this.$typedGetters['files/getFile'](path, files.items[0])
+      if (files) {
+        const path = files.path ? `gcodes/${files.path}` : 'gcodes'
 
-      if (file) {
-        this.loadFile(file)
+        const file: AppFile | undefined = this.$typedGetters['files/getFile'](path, files.items[0])
+
+        if (file) {
+          this.loadFile(file)
+        }
       }
     }
   }

src/components/widgets/job-queue/JobQueue.vue

@@ -245,11 +245,14 @@ export default class JobQueue extends Vue {
       hasFileDataTransferTypeInDataTransfer(event.dataTransfer, 'jobs')
     ) {
       const files = getFileDataTransferDataFromDataTransfer(event.dataTransfer, 'jobs')
-      const filePath = files.path ? `${files.path}/` : ''
-      const filenames = files.items
-        .map(file => `${filePath}${file}`)
 
-      SocketActions.serverJobQueuePostJob(filenames)
+      if (files) {
+        const filePath = files.path ? `${files.path}/` : ''
+        const filenames = files.items
+          .map(file => `${filePath}${file}`)
+
+        SocketActions.serverJobQueuePostJob(filenames)
+      }
     }
   }
 }

src/plugins/socketClient.ts

@@ -339,7 +339,7 @@ interface SocketApiResponse extends SocketResponseBase {
 
 interface SocketApiErrorResponse extends SocketResponseBase {
   id: number;
-  error: string | SocketError;
+  error: SocketError;
 }
 
 interface SocketNotificationResponse extends SocketResponseBase {

src/store/socket/actions.ts

@@ -1,6 +1,6 @@
 import type { ActionTree } from 'vuex'
 import { consola } from 'consola'
-import type { SocketState, SocketStatus } from './types'
+import type { SocketError, SocketState, SocketStatus } from './types'
 import type { RootState } from '../types'
 import { Globals } from '@/globals'
 import { SocketActions } from '@/api/socketActions'
@@ -30,6 +30,27 @@ const VALID_TRANSITIONS: Record<SocketStatus, readonly SocketStatus[]> = {
   ready: ['disconnected', 'connecting', 'authenticating']
 }
 
+const tryGetErrorMessageFromJson = (message: string): string | null => {
+  try {
+    // If our message contains JSON, we should try to parse it.
+    // This was a problem in old versions of Moonraker, not sure if it's still the case, but it doesn't hurt to be defensive here.
+    const messageAsObject = JSON.parse(message.replace(/'/g, '"'))
+
+    if (
+      messageAsObject !== null &&
+      typeof messageAsObject === 'object' &&
+      'message' in messageAsObject &&
+      typeof messageAsObject.message === 'string'
+    ) {
+      return messageAsObject.message
+    }
+  } catch {
+    // ignore and assume it's a plain string
+  }
+
+  return null
+}
+
 const getMoonrakerDatabase = async <T = Record<string, unknown>>(namespace: string) => {
   try {
     const response = await SocketActions.serverDatabaseGetItem<T>(undefined, namespace)
@@ -258,18 +279,16 @@ export const actions = {
    * Fired when the socket encounters an error. ws.onclose always follows and
    * drives the transition; here we just surface RPC error codes.
    */
-  async onSocketError ({ state, commit }, payload) {
-    if (state.status === 'ready' && payload.code >= 400 && payload.code < 500) {
-      // If our message contains json, we should try to parse it.
-      // This is pretty bad, should get moonraker to fix this response.
-      let message = ''
-      try {
-        const messageAsObject = JSON.parse(payload.message.replace(/'/g, '"')) as { message: string }
-
-        message = messageAsObject.message
-      } catch {
-        message = payload.message
-      }
+  async onSocketError ({ state, commit }, payload: SocketError) {
+    if (
+      state.status === 'ready' &&
+      payload.code >= 400 &&
+      payload.code < 500
+    ) {
+      const message = (
+        tryGetErrorMessageFromJson(payload.message) ||
+        payload.message
+      )
 
       EventBus.$emit(message, { type: 'error' })
     } else if (payload.code === 503) {

src/store/socket/state.ts

@@ -4,7 +4,6 @@ export const defaultState = (): SocketState => {
   return {
     status: 'initializing',
     acceptingNotifications: false,
-    error: null,
     connectionId: null
   }
 }

src/store/socket/types.ts

@@ -1,7 +1,6 @@
 export interface SocketState {
   status: SocketStatus;
   acceptingNotifications: boolean;
-  error: SocketError | null;
   connectionId: number | null;
 }
 

src/util/file-data-transfer.ts

@@ -1,4 +1,15 @@
 import { Globals } from '@/globals'
+import consola from 'consola'
+
+const isFileDataTransferData = (dataAsObject: unknown): dataAsObject is FileDataTransferData => (
+  dataAsObject !== null &&
+  typeof dataAsObject === 'object' &&
+  'path' in dataAsObject &&
+  typeof dataAsObject.path === 'string' &&
+  'items' in dataAsObject &&
+  Array.isArray(dataAsObject.items) &&
+  dataAsObject.items.every(item => typeof item === 'string')
+)
 
 export type FileDataTransferData = {
   path: string,
@@ -13,10 +24,20 @@ export const setFileDataTransferDataInDataTransfer = (dataTransfer: DataTransfer
   dataTransfer.setData(Globals.FILE_DATA_TRANSFER_TYPES[type], JSON.stringify(fileDataTransferData))
 }
 
-export const getFileDataTransferDataFromDataTransfer = (dataTransfer: DataTransfer, type: keyof typeof Globals.FILE_DATA_TRANSFER_TYPES) => {
+export const getFileDataTransferDataFromDataTransfer = (dataTransfer: DataTransfer, type: keyof typeof Globals.FILE_DATA_TRANSFER_TYPES): FileDataTransferData | null => {
   const data = dataTransfer.getData(Globals.FILE_DATA_TRANSFER_TYPES[type])
 
-  const fileDataTransferData = JSON.parse(data) as FileDataTransferData
+  try {
+    const dataAsObject = JSON.parse(data)
+
+    if (!isFileDataTransferData(dataAsObject)) {
+      throw new Error('Data in data transfer is not of type FileDataTransferData')
+    }
+
+    return dataAsObject
+  } catch (error) {
+    consola.warn(`Failed to parse data transfer data for type ${Globals.FILE_DATA_TRANSFER_TYPES[type]}`, data, error)
+  }
 
-  return fileDataTransferData
+  return null
 }

src/util/string-formatters.ts

@@ -47,8 +47,16 @@ const stringFormatters = () => {
     getStringArray: (value: string, separator = ';') => {
       if (value.startsWith('["') && value.endsWith('"]')) {
         try {
-          return JSON.parse(value) as string[]
+          const valueAsObject = JSON.parse(value)
+
+          if (
+            Array.isArray(valueAsObject) &&
+            valueAsObject.every(item => typeof item === 'string')
+          ) {
+            return valueAsObject
+          }
         } catch {
+          // ignore and fallback to splitting by separator
         }
       }
 

src/workers/parseGcode.ts

@@ -1,5 +1,5 @@
 /* eslint-disable no-fallthrough */
-import type { ArcMove, BBox, Layer, LinearMove, Move, Part, Point, PositioningMode } from '@/store/gcodePreview/types'
+import type { ArcMove, BBox, Layer, LinearMove, Move, Part, PositioningMode } from '@/store/gcodePreview/types'
 import isKeyOf from '@/util/is-key-of'
 import { pick } from 'lodash-es'
 import { split } from 'shlex'
@@ -63,6 +63,16 @@ const decimalRound = (a: number) => {
   return Math.round(a * 10000) / 10000
 }
 
+const isPolygonData = (data: unknown): data is [number, number][] => (
+  Array.isArray(data) &&
+  data
+    .every(x => (
+      Array.isArray(x) &&
+      x.length === 2 &&
+      x.every(y => typeof y === 'number')
+    ))
+)
+
 const parseGcode = (gcode: string, sendProgress: (filePosition: number) => void) => {
   const moves: Move[] = []
   const layers: Layer[] = []
@@ -115,17 +125,20 @@ const parseGcode = (gcode: string, sendProgress: (filePosition: number) => void)
           break
         case 'EXCLUDE_OBJECT_DEFINE':
           if ('polygon' in args && args.polygon) {
-            const polygonData = JSON.parse(args.polygon) as [number, number][]
-
-            const part: Part = {
-              polygon: polygonData
-                .map(([x, y]): Point => ({
-                  x,
-                  y
-                }))
+            try {
+              const data = JSON.parse(args.polygon)
+
+              if (isPolygonData(data)) {
+                const part: Part = {
+                  polygon: data
+                    .map(([x, y]) => ({ x, y }))
+                }
+
+                parts.push(part)
+              }
+            } catch {
+              // ignore invalid JSON
             }
-
-            parts.push(part)
           }
           break
         case 'SET_RETRACTION':