Skip to content

Error loading main module tries to build temp file, fails, reports wrong error #331

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
1 task done
dathbe opened this issue Apr 12, 2025 · 21 comments
Closed
1 task done

Comments

@dathbe
Copy link
Contributor

dathbe commented Apr 12, 2025

OS

Debian 10

NodeJS Version

22

MagicMirror² Version

2.31

Remote Control Version

2.5.3

Did you try using just Remote Control alone with MM?

  • I have and the error still happening

Description

MMM-Remote-Control tries to load /MMM-RAIN-MAP/MMM-RAIN-MAP.js, which fails initially, so it tries to build a temp.js file. Loading the reformatted temp.js also fails, which causes an error in the logs.

The error reported on the actual js file and the error reported on the temp.js are different, and only the latter gets reported in the log (which I do not believe is the real error).

temp.js is not deleted, so it just stays there.

Expected behavior

  1. Ideally, we could somehow tweak Remote-Control to load MMM-RAIN-MAP properly
  2. The proper error should be reported if not.
  3. The temp.js file should be cleaned up.

Current behavior

With module MMM-RAIN-MAP, MMM-Remote-Control tries to load the main js file, fails in node_helper.js at line 328.

The apparent error is with the very end of MMM-RAIN-MAP.js where there is an additional (Log) after the usual final }. I could not tell you why it's there, what it does, or why MMM-Remote-Control does not like it (the MMM-RAIN-MAP module works fine on its own).

[2025-04-12 13:29:28.265] [WARN]  ReferenceError: Log is not defined
0|mm  |     at Object.<anonymous> (~/MagicMirror/modules/MMM-RAIN-MAP/MMM-RAIN-MAP.js:28:158622)
0|mm  |     at Module._compile (node:internal/modules/cjs/loader:1569:14)
0|mm  |     at Module._extensions..js (node:internal/modules/cjs/loader:1722:10)
0|mm  |     at Module.load (node:internal/modules/cjs/loader:1296:32)
0|mm  |     at Module._load (node:internal/modules/cjs/loader:1115:12)
0|mm  |     at c._load (node:electron/js2c/node_init:2:18008)
0|mm  |     at TracingChannel.traceSync (node:diagnostics_channel:322:14)
0|mm  |     at wrapModuleLoad (node:internal/modules/cjs/loader:227:24)
0|mm  |     at Module.require (node:internal/modules/cjs/loader:1318:12)
0|mm  |     at require (node:internal/modules/helpers:136:16)
0|mm  |     at Class.loadModuleDefaultConfig (~/MagicMirror/modules/MMM-Remote-Control/node_helper.js:328:7)
0|mm  |     at ~/MagicMirror/modules/MMM-Remote-Control/node_helper.js:280:14
0|mm  |     at FSReqCallback.oncomplete (node:fs:199:5)

The above logging had to be added.

The catch block for line 328 says to build a temp.js file to hopefully correct some issues that apparently come from reading bundled apps. That temp.js file is created, but it's no use. Adding some logging shows a DIFFERENT error on the temp file than on the original file:

[2025-04-12 13:54:20.042] [WARN]  TypeError: Cannot read properties of undefined (reading 'style')
0|mm  |     at ~/MagicMirror/modules/temp.js:29:16027
0|mm  |     at ~/MagicMirror/modules/temp.js:29:149531
0|mm  |     at Object.<anonymous> (~/MagicMirror/modules/temp.js:29:158621)
0|mm  |     at Module._compile (node:internal/modules/cjs/loader:1569:14)
0|mm  |     at Module._extensions..js (node:internal/modules/cjs/loader:1722:10)
0|mm  |     at Module.load (node:internal/modules/cjs/loader:1296:32)
0|mm  |     at Module._load (node:internal/modules/cjs/loader:1115:12)
0|mm  |     at c._load (node:electron/js2c/node_init:2:18008)
0|mm  |     at TracingChannel.traceSync (node:diagnostics_channel:322:14)
0|mm  |     at wrapModuleLoad (node:internal/modules/cjs/loader:227:24)
0|mm  |     at Module.require (node:internal/modules/cjs/loader:1318:12)
0|mm  |     at require (node:internal/modules/helpers:136:16)
0|mm  |     at Class.loadModuleDefaultConfig (~/MagicMirror/modules/MMM-Remote-Control/node_helper.js:339:11)
0|mm  |     at ~/MagicMirror/modules/MMM-Remote-Control/node_helper.js:280:14
0|mm  |     at FSReqCallback.oncomplete (node:fs:199:5)

Again, the above logging had to be added for debug purposes.

After failing on the temp.js, vanilla Remote-Control reports the following error, which is both incorrect and not particularly helpful

[2025-04-12 13:29:28.422] [ERROR] [MMM-Remote-Control] Could not load main module js file. Error found: Cannot read properties of undefined (reading 'style')

(I confirmed this to be MMM-RAIN-MAP by some added logging, #330 .)

The temp.js file is never deleted.

So Remote-Control catches an error that (maybe?) it shouldn't catch.
Tries to build a temp.js file to get around an error that is not present.
Reports the wrong error because it reports the error on the temp.js.
And then doesn't delete the temp.js it creates.

Possible solution

Either be more lenient on the ending (Log) in the MMM-RAIN-MAP file, or more accurately report that error? (I tried to delete it from MMM-RAIN-MAP, but it breaks that module).

More carefully select which files get written to temp files?

Clean up temp files after attempt?

Steps to reproduce

Install MMM-RAIN-MAP
Install MMM-Remote-Control
Check logs

Log

See above

config.js

n/a

Additional info

No response

@dathbe
Copy link
Contributor Author

dathbe commented Apr 13, 2025

Additional thoughts on ways to address this:

Config option for something like ignoreModules where modules can be explicitly ignored.
Instead of having nested catch blocks with the same e error variable, have two different error variables and return either both or the first one (based on the real file instead of the temp.js.

Unfortunately, I don't even know what Remote-Control is trying to do when it "loads" a module, so I'm not a ton of help.

@KristjanESPERANTO
Copy link
Collaborator

As far as I understand it, it's about getting the default options and values of the modules. This is somehow different for the bundled modules than for the normal ones. I don't know if I'll be able to take a closer look soon. If someone else can do that, I would appreciate it.

@dathbe
Copy link
Contributor Author

dathbe commented Apr 14, 2025

Just to be clear, “bundled” means the default apps that come with vanilla MM?

Wouldn’t it be easier and more precise to identify theses by filepath instead of “if error, catch”?

@KristjanESPERANTO
Copy link
Collaborator

Just to be clear, “bundled” means the default apps that come with vanilla MM?

No, with "bundled" I mean modules like MMM-RAIN-MAP which are developed in TypeScript and have a build process to create the final file(s). Checkout the MMM-RAIN-MAP.js - it doesn't look like a normal module js file.

Wouldn’t it be easier and more precise to identify theses by filepath instead of “if error, catch”?

Identifying is not the problem. The problem is to load the defaults from those files.

@KristjanESPERANTO
Copy link
Collaborator

Can you please check if this issue also occurs if you don't use pm2? I can't reproduce it 🤔

@dathbe
Copy link
Contributor Author

dathbe commented Apr 14, 2025

No, with "bundled" I mean modules like MMM-RAIN-MAP

Oh, so the catch-then-create-temp.js is actually designed for modules like this. I'll have to look at it more closely and see if I can come up with a solution.

Can you please check if this issue also occurs if you don't use pm2? I can't reproduce it 🤔

I'd be happy to, but I don't actually know what the alternative method is.

@dathbe
Copy link
Contributor Author

dathbe commented Apr 14, 2025

I mean, changing line 334 to:

const newContent = `const Log = console;const document = navigator = window = {};document.createElement = function() { return {}; };\n${fs.readFileSync(filename, "utf8").replace('(Log)','')}`;

Stops throwing errors. It just removes the weird "(Log)" from the end of the file. I couldn't tell you if the file is properly loaded as I do not use Remote-Control for anything having to do with RAIN-MAP.

And yes, I confirmed that's the only instance of "(Log)" in the file. But that's obviously going to be hit or miss for other files of this type.

A slightly safer though more verbose way of doing it would be something like:

var fileContent = fs.readFileSync(filename, "utf8");
if (fileContent.endsWith('}(Log);\n')) {
  fileContent = fileContent.substring(0, fileContent.length - 7) + ';\n';
}
const newContent = `const Log = console;const document = navigator = window = {};document.createElement = function() { return {}; };\n${fileContent}`;

Sometime someone is going to have to explain to me why some javascript files have a semicolon at the end of every line and others don't.

@KristjanESPERANTO
Copy link
Collaborator

I'm on vacation for the next few days 😎 I'll take a look at it afterward.

@KristjanESPERANTO
Copy link
Collaborator

KristjanESPERANTO commented Apr 23, 2025

Thanks for the PR!

Unfortunately, I cannot recreate the problem. Not even with pm2. Since the manipulation of the bundled modules is already a bit hacky, I would like to understand the problem better before I merge the PR.

Can you please test with the following minimal config.js if the problem occurs on your system with it?

let config = {
  address: "0.0.0.0",
  port: 8080,
  basePath: "/",
  ipWhitelist: [],
  logLevel: ["INFO", "LOG", "WARN", "ERROR", "DEBUG"],
  modules: [
    {
      module: "MMM-RAIN-MAP",
      position: "top_left"
    },
    {
      module: "MMM-Remote-Control",
      position: "bottom_left"
    }
  ]
};

/** ************* DO NOT EDIT THE LINE BELOW ***************/
if (typeof module !== "undefined") {  module.exports = config; }

I'd be happy to, but I don't actually know what the alternative method is.

Run that in the command line:

cd ~/MagicMirror
npm run start

Sometime someone is going to have to explain to me why some javascript files have a semicolon at the end of every line and others don't.

Most developers use semicolons at the end of every line in JavaScript for clarity and to avoid issues with Automatic Semicolon Insertion (ASI), while others skip them, trusting ASI to handle it. I would say that it's a matter of coding style preference.

@dathbe
Copy link
Contributor Author

dathbe commented Apr 23, 2025

Seen. Will do when I have the opportunity and will report back.

@KristjanESPERANTO
Copy link
Collaborator

I just took a closer look at it, loading the defaults from bundled modules does not work (nor with or without your PR). We have to come up with something else.

@dathbe
Copy link
Contributor Author

dathbe commented Apr 23, 2025

I just took a closer look at it, loading the defaults from bundled modules does not work (nor with or without your PR). We have to come up with something else.

But at least the error message is gone! ;-)

@dathbe
Copy link
Contributor Author

dathbe commented Apr 24, 2025

Ok, I think I have another solution conceptually, but I don't know if it applies broadly to all "bundled" modules.

Most modules take the basic form of:

Module.register("MMM-ModuleName", {
  //<all the code>
});

MMM-RAIN-MAP, on the other hand, has the Module.register wrapped in a function:

! function(t) {
  //<several functions and variables defined>
  Module.register("MMM-RAIN-MAP", {
    //<all the code>
  });
}(Log);

If you delete all of the framing stuff to get it back down to just the Module.register, MMM-Remote-Control will properly grab the defaults. There must be some way to extract just the Module.register from the function wrapper?

I've truly just taught myself enough javascript to muddle through patching modules, so this is not my area of expertise.

One thing that would work here is something like:

fileContent = 'Module.register' + fileContent.split('Module.register')[1].split(`}${fileContent.split('}')[fileContent.split('}').length - 1]}`)[0]

(translated: look for the phrase Module.register, take everything after that, remove the final } (the one that closes out the wrapper function) and everything after, and then put Module.register back onto the start of the file.)

This would come right before
const newContent = `const Log = console;const document = navigator = window = {};document.createElement = function() { return {}; };\n${fileContent}`;

But i don't know if that would be AT ALL generalizable to other "bundled" modules.

@dathbe
Copy link
Contributor Author

dathbe commented Apr 24, 2025

A new commit incorporating this has been pushed in #333.
As a side note, I have checked that, if there are functions and other extraneous material after the Module.register block (such that they will not be removed from the newContent, this code will still accurately grab the defaults.

@KristjanESPERANTO
Copy link
Collaborator

A creative approach, but unfortunately the defaults are not loaded. You can see that if you use a minimal config like above and open the settings for MMM-RAIN-Map here: http://localhost:8080/remote.html#settings-menu

Image

@dathbe
Copy link
Contributor Author

dathbe commented Apr 24, 2025

Interesting. I was testing by adding some logging in node_helper and finding that the defaults do make it to that stage. Didn't know there was such a page. I'll do more testing.

@dathbe
Copy link
Contributor Author

dathbe commented Apr 24, 2025

Are you sure that page lists defaults and not just user-set options? Here's what mine looks like for MMM-Remote-Control and a different (non-bundle) module:

Image

Image

No defaults listed there either. Doesn't mean they're not loaded.

@KristjanESPERANTO
Copy link
Collaborator

Odd, because I got the defaults with a minimal config like this:

    {
      module: 'MMM-MyScoreboard',
      position: 'top_right',
      config: {}
    },

Image

I am in the process of working on a quite different approach. Maybe I'll manage to continue working on it in the next few days.

@KristjanESPERANTO
Copy link
Collaborator

Should be fixed with version 3.1.8 🙂 Please check.

The default settings for the bundled modules are now retrieved from the frontend. This is not yet perfect, but better than before.

@dathbe
Copy link
Contributor Author

dathbe commented Apr 28, 2025

I'm not getting an error anymore, but I did get this Log message:

Could not get defaults for MMM-RAIN-MAP. See #331.

@KristjanESPERANTO
Copy link
Collaborator

As release 3.1.8 has solved/changed the situation, I am closing this issue. Thanks again for your helpful input! 🙂

I opened issue #335 for a related issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants