megahertz / electron-log

Simple logging module Electron/Node.js/NW.js application. No dependencies. No complicated configuration.
MIT License
1.32k stars 128 forks source link

Renderer throws error trying to clone uncloneable object for serialisation #342

Closed UberMouse closed 1 year ago

UberMouse commented 1 year ago

Just updating our Electron app to v5 beta now, running into an issue trying to serialize some objects. Specifically, objects containing an xstate interpreter which isn't cloneable/serializable. Whenever one is logged our React app crashes as electron-log throws an error. Adding a try/catch in the preload around the ipcRenderer.send('__ELECTRON_LOG__', message); call stops the crash. The crash is happening in the electron serializer that ipcRenderer.send calls. Electron version 21.3.1

From looking at the docs, there doesn't seem to be a better solution? I thought maybe I could solve it with a transform that stripped those objects out, but that doesn't seem to be possible in the renderer process?

UberMouse commented 1 year ago

I've also had some problems with the toJSON/toString methods in the object.js transform throwing errors on similar objects. So I've patched those to have a try/catch as well. Patch for reference

diff --git a/src/main/transforms/object.js b/src/main/transforms/object.js
index 3b97493bd67060bc063f2651236fc59777dd598f..d3e8c593db27d7bb5d2dc5c8bb4fe01b0335636a 100644
--- a/src/main/transforms/object.js
+++ b/src/main/transforms/object.js
@@ -54,7 +54,12 @@ module.exports = {
   },

   toJSON({ data }) {
-    return JSON.parse(JSON.stringify(data, createSerializer()));
+    try {
+      return JSON.parse(JSON.stringify(data, createSerializer()));
+    } catch(e) {
+      console.error("failed to call toJSON", e);
+      return "";
+    }
   },

   toString({ data, transport }) {
@@ -75,7 +80,12 @@ module.exports = {
       }
     });

-    return util.formatWithOptions(inspectOptions, ...simplifiedData);
+    try {
+      return util.formatWithOptions(inspectOptions, ...simplifiedData);
+    } catch (e) {
+      console.error("failed to call toString", e);
+      return "";
+    }
   },
 };

diff --git a/src/renderer/preload.js b/src/renderer/preload.js
index a272c814ce3a28fd0b75d449c593a0cbdfc9e174..e3cbafe117818562079383b113d34aba830b9eef 100644
--- a/src/renderer/preload.js
+++ b/src/renderer/preload.js
@@ -35,7 +35,9 @@ function initialize({ contextBridge, ipcRenderer }) {

   const __electronLog = {
     sendToMain(message) {
-      ipcRenderer.send('__ELECTRON_LOG__', message);
+      try {
+        ipcRenderer.send('__ELECTRON_LOG__', message);
+      } catch (e) {}
     },

     log(...data) {
megahertz commented 1 year ago

Thank you for the report. I'm going to release a fix in a few days.

megahertz commented 1 year ago

Do you use xstate in the main process too? I've never seen an issue with that part before. There are no too much changes in transformers since v4.

megahertz commented 1 year ago

Now such an error handler correctly. I have no time to dive into xstate to find which data structure produces clone error. If someone could helps me with that I can change serializer to handle such a data properly.

UberMouse commented 1 year ago

Do you use xstate in the main process too?

Yes. I haven't noticed issues in the main process with it, but we don't log any xstate interpreters in the main process. I just ported our existing patch from v4 to v5 and we had try/catch around toJSON/toString since they used to be used in the renderer.