Skip to content
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

loadLua() not splitting lines correctly in connection.js #465

Closed
yiwei138 opened this issue Oct 31, 2020 · 2 comments · Fixed by #467
Closed

loadLua() not splitting lines correctly in connection.js #465

yiwei138 opened this issue Oct 31, 2020 · 2 comments · Fixed by #467
Labels

Comments

@yiwei138
Copy link

yiwei138 commented Oct 31, 2020

  • node-resque version: 8.0.0

  • Node.js Version: 14.15.0

  • OS: Windows 10

  • Error:

2020-10-31T17:13:50.951Z - error: error from task name=task:this.connection.redis.popAndStoreJob is not a function queue=default worker=1 stacktrace=TypeError: this.connection.redis.popAndStoreJob is not a function
    at Worker.getJob (D:\ABC\node_modules\node-resque\dist\core\worker.js:256:73)
    at Worker.poll (D:\ABC\node_modules\node-resque\dist\core\worker.js:105:43)
    at Worker.start (D:\ABC\node_modules\node-resque\dist\core\worker.js:59:18)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at MultiWorker.startWorker (D:\ABC\node_modules\node-resque\dist\core\multiWorker.js:93:9)
    at MultiWorker.checkWorkers (D:\ABC\node_modules\node-resque\dist\core\multiWorker.js:161:13)
    at MultiWorker.checkWrapper (D:\ABC\node_modules\node-resque\dist\core\multiWorker.js:186:42)
2020-10-31T17:13:50.893Z - error: error from task name=task:Unexpected number in JSON at position 23 worker=1 stacktrace=SyntaxError: Unexpected number in JSON at position 23
    at JSON.parse (<anonymous>)
    at Connection.loadLua (D:\ABC\node_modules\node-resque\dist\core\connection.js:84:35)
    at connectionTestAndLoadLua (D:\ABC\Server\node_modules\node-resque\dist\core\connection.js:40:22)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at Connection.connect (D:\ABC\Server\node_modules\node-resque\dist\core\connection.js:49:13)
    at Worker.connect (D:\ABC\node_modules\node-resque\dist\core\worker.js:50:9)
    at MultiWorker.startWorker (D:\ABC\node_modules\node-resque\dist\core\multiWorker.js:92:9)
    at MultiWorker.checkWorkers (D:\ABC\node_modules\node-resque\dist\core\multiWorker.js:161:13)
    at MultiWorker.checkWrapper (D:\ABC\node_modules\node-resque\dist\core\multiWorker.js:186:42)
  • For actionhero the tasks will raise exceptions and will not be run

  • Fix:

node-resque/dist/core/connection.js:
loadLua() {
        const luaDir = path.join(__dirname, "..", "..", "lua");
        const files = fs.readdirSync(luaDir);
        for (const i in files) {
            const file = files[i];
            const { name } = path.parse(file);
            const contents = fs.readFileSync(path.join(luaDir, file)).toString();
            const lines = contents.split('\n');  <===== The error is gone when change to '\n' instead of os.EOL
            const encodedMetadata = lines[0].replace(/^-- /, "");
            const metadata = JSON.parse(encodedMetadata);
            this.redis.defineCommand(name, {
                numberOfKeys: metadata.numberOfKeys,
                lua: contents,
            });
        }
    }
  • It seems that the lines are not split correctly when using os.EOL, I'm not sure is it my com specifically or its Win10 prob
@evantahler
Copy link
Member

It's likely the case since since the lua files were authored in a mac/linux environment, the \n line break is baked in. I'll update this method to split on both \r and \n to be backwards compatible and future-proof.

I looked into also running out test suite on Windows, but it doesn't currently seem possible to run node.js tests and install redis on CircleCI windows machines

@evantahler
Copy link
Member

evantahler commented Oct 31, 2020

V8.0.1 of node-resque published with this fix!
https://github.com/actionhero/node-resque/releases/tag/v8.0.1

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

Successfully merging a pull request may close this issue.

2 participants