fix: cloneing objects

Did not really want to use a library for this but i have used for it to go away for now. I think having the context and task to further debug errors are beneficial. Maybe we can make it opt-in in the future. But for having circular dependencies inside the context, this was causing problems without a deep copy. fix #571, fix #569, fix #538
listr2 · Nov 2, 2021 · 15e8317 · 15e8317 · jurijzahn8019 · Nov 2, 2021
1 parent b629d0e
commit 15e8317
Show file tree

Hide file tree

Showing 8 changed files with 535 additions and 225 deletions.
diff --git a/package.json b/package.json
@@ -64,6 +64,7 @@
   ],
   "dependencies": {
     "cli-truncate": "^2.1.0",
+    "clone": "^2.1.2",
     "colorette": "^2.0.16",
     "log-update": "^4.0.0",
     "p-map": "^4.0.0",
@@ -73,15 +74,16 @@
   },
   "devDependencies": {
     "@cenk1cenk2/cz-cc": "^1.4.9",
-    "@cenk1cenk2/eslint-config": "^0.4.50",
+    "@cenk1cenk2/eslint-config": "^1.0.5",
+    "@types/clone": "^2.1.1",
     "@types/jest": "^27.0.2",
     "@types/node": "^16.7.8",
     "@types/rewire": "^2.5.28",
     "@types/through": "^0.0.30",
     "@types/wrap-ansi": "^3.0.0",
     "delay": "^5.0.0",
     "enquirer": "^2.3.6",
-    "eslint": "^7",
+    "eslint": "^8.1.0",
     "jest": "^27.3.1",
     "jest-mock-process": "^1.4.1",
     "lint-staged": "^11.2.4",

diff --git a/src/lib/task-wrapper.ts b/src/lib/task-wrapper.ts
@@ -55,7 +55,7 @@ export class TaskWrapper<Ctx, Renderer extends ListrRendererFactory> {
 
   /** Report a error in process for error collection. */
   public report (error: Error, type: ListrErrorTypes): void {
-    this.errors.push(new ListrError<Ctx>(error, type, cloneObject(this.task.listr.ctx), this.task))
+    this.errors.push(new ListrError<Ctx>(error, type, cloneObject(this.task.listr.ctx), cloneObject(this.task)))
 
     this.task.message$ = { error: error.message ?? this.task?.title ?? 'Task with no title.' }
   }

diff --git a/src/renderer/default.renderer.ts b/src/renderer/default.renderer.ts
@@ -115,21 +115,21 @@ export class DefaultRenderer implements ListrRenderer {
      */
     formatOutput?: 'truncate' | 'wrap'
   } = {
-    indentation: 2,
-    clearOutput: false,
-    showSubtasks: true,
-    collapse: true,
-    collapseSkips: true,
-    showSkipMessage: true,
-    suffixSkips: true,
-    collapseErrors: true,
-    showErrorMessage: true,
-    suffixRetries: true,
-    lazy: false,
-    showTimer: false,
-    removeEmptyLines: true,
-    formatOutput: 'truncate'
-  }
+      indentation: 2,
+      clearOutput: false,
+      showSubtasks: true,
+      collapse: true,
+      collapseSkips: true,
+      showSkipMessage: true,
+      suffixSkips: true,
+      collapseErrors: true,
+      showErrorMessage: true,
+      suffixRetries: true,
+      lazy: false,
+      showTimer: false,
+      removeEmptyLines: true,
+      formatOutput: 'truncate'
+    }
 
   /** per task options for the default renderer */
   public static rendererTaskOptions: {

diff --git a/src/renderer/simple.renderer.ts b/src/renderer/simple.renderer.ts
@@ -49,58 +49,58 @@ export class SimpleRenderer implements ListrRenderer {
   public eventTypeRendererMap: Partial<{
     [P in ListrEventType]: (t: Task<any, typeof SimpleRenderer>, event: ListrEventFromType<P>) => void
   }> = {
-    [ListrEventType.SUBTASK]: (task) => {
-      if (task.hasTitle()) {
+      [ListrEventType.SUBTASK]: (task) => {
+        if (task.hasTitle()) {
         // if Task has subtasks where we want to log the group indication
-        this.log(`${colorette.blue(figures.pointer)} ${task.title}`)
-      }
-
-      if (task.hasSubtasks()) {
-        this.render(task.subtasks)
-      }
-    },
-    [ListrEventType.STATE]: (task) => {
-      if (task.isCompleted() && task.hasTitle()) {
+          this.log(`${colorette.blue(figures.pointer)} ${task.title}`)
+        }
+
+        if (task.hasSubtasks()) {
+          this.render(task.subtasks)
+        }
+      },
+      [ListrEventType.STATE]: (task) => {
+        if (task.isCompleted() && task.hasTitle()) {
         // The title is only logged at the end of the task execution
-        this.log(`${colorette.green(figures.tick)} ${task.title}`)
-      }
-    },
-    [ListrEventType.DATA]: (task, event) => {
+          this.log(`${colorette.green(figures.tick)} ${task.title}`)
+        }
+      },
+      [ListrEventType.DATA]: (task, event) => {
       // ! This is where it gets dirty
       // * We want the prompt to stay visible after confirmation
-      if (task.isPrompt() && !String(event.data).match(/^\n$/)) {
-        logUpdate(`${event.data}`)
-      } else {
-        this.log(`${figures.pointerSmall} ${event.data}`)
-      }
-    },
-    [ListrEventType.MESSAGE]: (task, event) => {
-      if (event.data.error) {
+        if (task.isPrompt() && !String(event.data).match(/^\n$/)) {
+          logUpdate(`${event.data}`)
+        } else {
+          this.log(`${figures.pointerSmall} ${event.data}`)
+        }
+      },
+      [ListrEventType.MESSAGE]: (task, event) => {
+        if (event.data.error) {
         // error message
-        const title = SimpleRenderer.formatTitle(task)
+          const title = SimpleRenderer.formatTitle(task)
 
-        this.log(`${colorette.red(figures.cross)}${title}: ${event.data.error}`)
-      } else if (event.data.skip) {
+          this.log(`${colorette.red(figures.cross)}${title}: ${event.data.error}`)
+        } else if (event.data.skip) {
         // Skip message
-        const title = SimpleRenderer.formatTitle(task)
-        const skip = task.title !== event.data.skip ? `: ${event.data.skip}` : ''
+          const title = SimpleRenderer.formatTitle(task)
+          const skip = task.title !== event.data.skip ? `: ${event.data.skip}` : ''
 
-        this.log(`${colorette.yellow(figures.arrowDown)}${title} [${colorette.yellow(`skipped${skip}`)}]`)
-      } else if (event.data.rollback) {
+          this.log(`${colorette.yellow(figures.arrowDown)}${title} [${colorette.yellow(`skipped${skip}`)}]`)
+        } else if (event.data.rollback) {
         // rollback message
-        const title = SimpleRenderer.formatTitle(task)
+          const title = SimpleRenderer.formatTitle(task)
 
-        this.log(`${colorette.red(figures.arrowLeft)}${title}: ${event.data.rollback}`)
-      } else if (event.data.retry) {
+          this.log(`${colorette.red(figures.arrowLeft)}${title}: ${event.data.rollback}`)
+        } else if (event.data.retry) {
         // Retry Message
-        const title = SimpleRenderer.formatTitle(task)
+          const title = SimpleRenderer.formatTitle(task)
 
-        this.log(`[${colorette.yellow(`${event.data.retry.count}`)}]${title}`)
+          this.log(`[${colorette.yellow(`${event.data.retry.count}`)}]${title}`)
+        }
       }
-    }
     // * We do not log out initial title. Only the final one.
     // [ListrEventType.TITLE]: (t, e) => this.renderTitle(t, e),
-  }
+    }
 
   constructor (public readonly tasks: Task<any, typeof SimpleRenderer>[], public options: typeof SimpleRenderer['rendererOptions']) {
     this.options = { ...SimpleRenderer.rendererOptions, ...options }

diff --git a/src/renderer/verbose.renderer.ts b/src/renderer/verbose.renderer.ts
@@ -41,10 +41,10 @@ export class VerboseRenderer implements ListrRenderer {
          */
     options?: any
   } = {
-    useIcons: false,
-    logEmptyTitle: true,
-    logTitleChange: true
-  }
+      useIcons: false,
+      logEmptyTitle: true,
+      logTitleChange: true
+    }
   /** per task options for the verbose renderer */
   public static rendererTaskOptions: never
   private logger: Logger

diff --git a/src/utils/general.ts b/src/utils/general.ts
@@ -1,8 +1,8 @@
-import { serialize, deserialize } from 'v8'
+import * as clone from 'clone'
 
 /**
  * Deep clones a object in the most easiest manner.
  */
 export function cloneObject<T extends Record<PropertyKey, any>> (obj: T): T {
-  return deserialize(serialize(obj))
+  return clone(obj)
 }
diff --git a/tests/error-collection.e2e-spec.ts b/tests/error-collection.e2e-spec.ts
@@ -122,6 +122,37 @@ describe('error collection', () => {
     expect(task.err[0]).toMatchObject({ message, ctx })
   })
 
+  it('should save the context on error while having circular dependencies', async () => {
+    const message = '1'
+    const task = new Listr(
+      [
+        {
+          task: (ctx): void => {
+            ctx.test = true
+            ctx.myself = ctx
+
+            throw new Error(message)
+          }
+        }
+      ],
+      { renderer: 'silent', exitOnError: true }
+    )
+
+    let result: any
+    let crash: Error
+    try {
+      result = await task.run()
+    } catch (e: any) {
+      crash = e
+    }
+
+    expect(result).toBeFalsy()
+    expect(crash).toBeTruthy()
+    expect(task.err.length).toBe(1)
+    expect(task.err[0].message).toBe(message)
+    expect(task.err[0].ctx).toMatchObject({ test: true, myself: { test: true } })
+  })
+
   it('should collect all the errors from subtasks and fail with subtask error while subtask has exit on error', async () => {
     const task = new Listr(
       [