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

nodejs unit-http misses options in createServer function #1043

Closed
stefanhuber opened this issue Jan 3, 2024 · 2 comments · Fixed by #1091
Closed

nodejs unit-http misses options in createServer function #1043

stefanhuber opened this issue Jan 3, 2024 · 2 comments · Fixed by #1091
Assignees
Labels
z-bug 🐞 z-node-js Language-Module for JavaScript (Node.js)

Comments

@stefanhuber
Copy link

I tried to run strapi inside nginx unit. The startup of strapi doesn't work because of the following error:

TypeError [ERR_INVALID_ARG_TYPE]: The "listener" argument must be of type function. Received an instance of Object
    at checkListener (node:events:270:3)
    at _addListener (node:events:550:3)
    at Server.addListener (node:events:609:10)
    at new Server (/usr/local/lib/node_modules/unit-http/http_server.js:426:14)
    at Object.createServer (/usr/local/lib/node_modules/unit-http/http.js:15:12)
    at Module.createHTTPServer (/var/www/node_modules/@strapi/strapi/dist/services/server/http-server.js:16:40)
    at Module.createServer (/var/www/node_modules/@strapi/strapi/dist/services/server/index.js:26:35)
    at new Strapi (/var/www/node_modules/@strapi/strapi/dist/Strapi.js:130:27)
    at initFn (/var/www/node_modules/@strapi/strapi/dist/Strapi.js:466:18)
    at Object.<anonymous> (/var/www/index.js:2:1) {
  code: 'ERR_INVALID_ARG_TYPE'
}

Strapi uses options with the http.createServer function as seen here in the source code. The node signature of the createServer function looks like this: http.createServer([options][, requestListener]). Looking at the http module inside the unit-http package the createServer function is missing the options argument.

Can this be solved somehow? How could strapi be started otherwise within nginx unit?

Nginx Unit has the following config.json:

{
    "listeners": {
        "*:1337": {
            "pass": "routes/main"
        }
    },
    "routes": {
        "main": [
            {
                "action": {
                    "pass": "applications/app"
                }
            }
        ]
    },
    "applications": {
        "app": {
            "type": "external",
            "working_directory": "/var/www/",
            "executable": "/usr/bin/env",
            "arguments": [
                "node",
                "--loader",
                "unit-http/loader.mjs",
                "--require",
                "unit-http/loader",
                "index.js"
            ]
        }
    }
}
@stefanhuber
Copy link
Author

I could create a fix by using patch-package with the following steps:

  • Install unit-http and patch-package as npm dependencies: npm i unit-http patch-package --save
  • Add "postinstall": "patch-package" under "scripts" inside package.json
  • Edit the http.js file inside node_modules/unit-http by adding options as the first argument to the createServer function:
    function createServer (options, requestHandler) {
     return new Server(requestHandler);
    }
    
  • Apply the patch: npx patch-package unit-http

As a current workaround this is ok. Although I think this might not be a sustainable approach. The options could be ignored in the case of our strapi deployment.

@callahad
Copy link
Collaborator

callahad commented Jan 5, 2024

Looks like the options argument was added in Node.js v9.6.0 (2018-02-22) / v8.12.0 (2018-09-11) in nodejs/node#15752 and we missed it when implementing our shim for http.createServer.

We should fix this in Unit, but the patch-package workaround should be fine for your use case until we can get a release out with a proper fix.

@danielledeleo danielledeleo added the z-node-js Language-Module for JavaScript (Node.js) label Jan 15, 2024
@javorszky javorszky self-assigned this Jan 24, 2024
javorszky added a commit to javorszky/unit that referenced this issue Jan 25, 2024
Closes nginx#1043

Unit-http have not kept up with the signature of nodejs's http package
development. Nodejs allows an optional `options` object to be passed to
the `createServer` function, we didn't. This resulted in function
signature errors when user code that did make use of the options arg
tried to call unit's replaced function.

This change changes the signature to be more in line with how nodejs
does it discarding it and printing a message to stdout.
javorszky added a commit to javorszky/unit that referenced this issue Jan 26, 2024
Closes nginx#1043

Unit-http have not kept up with the signature of nodejs's http package
development. Nodejs allows an optional `options` object to be passed to
the `createServer` function, we didn't. This resulted in function
signature errors when user code that did make use of the options arg
tried to call unit's replaced function.

This change changes the signature to be more in line with how nodejs
does it discarding it and printing a message to stdout.
javorszky added a commit to javorszky/unit that referenced this issue Jan 29, 2024
Closes nginx#1043

Unit-http have not kept up with the signature of nodejs's http package
development. Nodejs allows an optional `options` object to be passed to
the `createServer` function, we didn't. This resulted in function
signature errors when user code that did make use of the options arg
tried to call unit's replaced function.

This change changes the signature to be more in line with how nodejs
does it discarding it and printing a message to stdout.
javorszky added a commit to javorszky/unit that referenced this issue Jan 29, 2024
Closes nginx#1043

Unit-http have not kept up with the signature of nodejs's http package
development. Nodejs allows an optional `options` object to be passed to
the `createServer` function, we didn't. This resulted in function
signature errors when user code that did make use of the options arg
tried to call unit's replaced function.

This change changes the signature to be more in line with how nodejs
does it discarding it and printing a message to stdout.
javorszky added a commit to javorszky/unit that referenced this issue Jan 29, 2024
Closes nginx#1043

Unit-http have not kept up with the signature of nodejs's http package
development. Nodejs allows an optional `options` object to be passed to
the `createServer` function, we didn't. This resulted in function
signature errors when user code that did make use of the options arg
tried to call unit's replaced function.

This change changes the signature to be more in line with how nodejs
does it discarding it and printing a message to stdout.
javorszky added a commit to javorszky/unit that referenced this issue Feb 8, 2024
Closes nginx#1043

Unit-http have not kept up with the signature of nodejs's http package
development. Nodejs allows an optional `options` object to be passed to
the `createServer` function, we didn't. This resulted in function
signature errors when user code that did make use of the options arg
tried to call unit's replaced function.

This change changes the signature to be more in line with how nodejs
does it discarding it and printing a message to stdout.
javorszky added a commit to javorszky/unit that referenced this issue Feb 14, 2024
Closes nginx#1043

Unit-http have not kept up with the signature of nodejs's http package
development. Nodejs allows an optional `options` object to be passed to
the `createServer` function, we didn't. This resulted in function
signature errors when user code that did make use of the options arg
tried to call unit's replaced function.

This change changes the signature to be more in line with how nodejs
does it discarding it and printing a message to stdout.
javorszky added a commit to javorszky/unit that referenced this issue Feb 14, 2024
Unit-http have not kept up with the signature of nodejs's http package
development. Nodejs allows an optional `options` object to be passed to
the `createServer` function, we didn't. This resulted in function
signature errors when user code that did make use of the options arg
tried to call unit's replaced function.

This change changes the signature to be more in line with how nodejs
does it discarding it and printing a message to stdout.

Closes: nginx#1043
javorszky added a commit that referenced this issue Feb 14, 2024
* Take options as well as requestListener

Unit-http have not kept up with the signature of nodejs's http package
development. Nodejs allows an optional `options` object to be passed to
the `createServer` function, we didn't. This resulted in function
signature errors when user code that did make use of the options arg
tried to call unit's replaced function.

This change changes the signature to be more in line with how nodejs
does it discarding it and printing a message to stdout.

* Add test file to start node application with options

* Add changes to docs/changes.xml

Closes: #1043
andrey-zelenkov pushed a commit that referenced this issue Feb 27, 2024
* Take options as well as requestListener

Unit-http have not kept up with the signature of nodejs's http package
development. Nodejs allows an optional `options` object to be passed to
the `createServer` function, we didn't. This resulted in function
signature errors when user code that did make use of the options arg
tried to call unit's replaced function.

This change changes the signature to be more in line with how nodejs
does it discarding it and printing a message to stdout.

* Add test file to start node application with options

* Add changes to docs/changes.xml

Closes: #1043
pkillarjun pushed a commit to pkillarjun/unit that referenced this issue May 29, 2024
* Take options as well as requestListener

Unit-http have not kept up with the signature of nodejs's http package
development. Nodejs allows an optional `options` object to be passed to
the `createServer` function, we didn't. This resulted in function
signature errors when user code that did make use of the options arg
tried to call unit's replaced function.

This change changes the signature to be more in line with how nodejs
does it discarding it and printing a message to stdout.

* Add test file to start node application with options

* Add changes to docs/changes.xml

Closes: nginx#1043
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
z-bug 🐞 z-node-js Language-Module for JavaScript (Node.js)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants