node-red/nrlint

Node red tab hogging the processor in the browser

colinl opened this issue · 9 comments

Current Behavior

Having upgraded from nrlint 1.0.2 to 1.1.0 the node-red tab is hogging the browser on Firefox and Chrome. Rollback to 1.0.2 restores normal operation.
Offending flow file shared privately.

Expected Behavior

No response

Steps To Reproduce

No response

Example flow

paste your flow here

Environment

  • nrlint version: 1.1.0
  • Node-RED version: 3.0.1
  • Node.js version: 16.16.0
  • npm version: 8.11.0
  • Platform/OS: Ubuntu 20.04
  • Browser: FF and Chrome

I thought it was just me.
Firefox 103 on Linux, an entire CPU core is maxed out

As an additional, it doesn't actually performing any linting either

I suspect the reason that it isn't doing any linting is because the process doing the linting is looping permanently trying, and failing, to parse a piece of code somewhere.
@My-Random-Thoughts if you have a short flow showing the problem then that might be helpful.

I have a test environment and removed all my flows and tabs but it was still maxing out.
I can send someone the flow.json file, but I not sure it's a flow specific issue

For me it was ok with an empty flows file (I think). What happens if you delete the flows file? In fact rename it rather than deleting it. Make sure the startup log then says it is creating a new flows file.

Um, maybe it is the flow file.
I renamed my current, restarted the docker container and it created a new flow.json file.
I created a couple of test flows, all looking OK
I imported some of the example flows - didn't know they were there! - All good
Imported my original file (via copy/paste of the json) and it maxed out

@colinl thank you for sharing your flow with my on the forum.

I can recreate the issue and have narrowed it down to the function-eslint rule.

In fact, I've narrowed it down a bit further than that...

First I identified one node in your flow that would cause the 100% CPU if present. It contains the following code:

const panels = msg.payload.panels
for (let i = 0; i < panels.length; i++) {
    const targets = panels[i].targets
    for (let j = 0; j < targets.length; j++) {
        node.send({ payload: `${targets[j].rawSql}\n` })
    }
}

return null;

I then narrowed it down to one line in that code:

        node.send({ payload: `${targets[j].rawSql}\n` })

I then narrowed it down to the \n in that line.

Quite why the presence of \n in that line is causing ESLint to get stuck, I have no idea. But its a start.

Wow, I can confirm this. In my test environment, I have one single \n string in a function.
I removed it, deployed and refreshed the page. All good now.

Very bizarre!

Have raised an upstream issue with eslint4b - the library we use to allow eslint to run in the browser.

Curiously, eslint@7.32.0 (the same version as eslint4b we have in nrlint) does not have a problem with this code.

mysticatea/eslint4b#17

Given the upstream project appears to have been abandoned, we are going to have to rethink how we use eslint in the browser.

We might have to fall back to doing it in the runtime side only - with some api in place for the browser linter to call it. That isn't ideal as currently all of the rules are written without knowledge of where they are running.