logux/server

Do not attach 'request' listener if custom http server is provided

g12i opened this issue Β· 6 comments

g12i commented

When I'm passing a custom HTTP server to Logux I would like to have full control over my routes.

import { Server as LoguxServer } from '@logux/server'
import http from 'http'
import polka from 'polka'

const server = http.createServer()

const app = polka({ server })
app.get('/status', (req, res) => res.end('OK'))

const loguxServer = new LoguxServer()

app.listen()
await loguxServer.listen()

This doesn't work and I have to detach it manually.

await loguxServer.listen()

let [polkaListener, loguxListener] = server.listeners('request')
server.off('request', loguxListener as any)

https://github.com/logux/server/blob/main/base-server/index.js#L400

ai commented

We call request for health check feature.

  1. The proper API to own HTTP callbacks is loguxServer.http(). Am I right that you can’t use because you want to use syntax sugar API like polka? Can we add something to make it compatible with polka?
  2. Maybe you have in mind some another API to keep Logux server health check and another request processor?
g12i commented

I imagine this passing custom http server is an advanced use case, where people are aware what they are doing. Would adding a note in documentation that Logux requires GET /health endpoint be enough?

I need to use some advanced connect middlewares. This is entirely possible, as long as Logux is not listening on its own. In my view it's better to allow utilising well-known patterns, as oppose to trying do everything in Logux itself. What do you think?

ai commented

We can add ignoreNonWsRequest (I will think about a better name) to Server options.

ai commented

I added disableHttpServer to next branch 5e4210e

You can switch from npm version to next from GitHub to test it:

yarn remove @logux/server && yarn add @logux/server@logux/server#next
g12i commented

Hi @ai. Sorry for late response, we were busy with other features. I'm afraid I cannot test it locally:

❯ yarn add @logux/server@logux/server#next
➀ YN0000: β”Œ Resolution step
➀ YN0013: β”‚ @logux/server@https://github.com/logux/server.git#commit=2f8deab4c59941074177ec08ae08a61cc33da9bf can't be found in the cache and will be fetched from GitHub
➀ YN0032: β”‚ fsevents@npm:2.3.2: Implicit dependencies on node-gyp are discouraged
➀ YN0013: β”‚ @logux/server@https://github.com/logux/server.git#commit=2f8deab4c59941074177ec08ae08a61cc33da9bf can't be found in the cache and will be fetched from the remot
➀ YN0001: β”‚ Error: @logux/server@https://github.com/logux/server.git#commit=2f8deab4c59941074177ec08ae08a61cc33da9bf: Assertion failed: Unsupported workflow
    at /Users/wojciech/Projects/akeero-logux/.yarn/releases/yarn-3.2.1.cjs:427:1576
    at async ar.mktempPromise (/Users/wojciech/Projects/akeero-logux/.yarn/releases/yarn-3.2.1.cjs:319:63575)
    at async /Users/wojciech/Projects/akeero-logux/.yarn/releases/yarn-3.2.1.cjs:424:16
    at async ar.mktempPromise (/Users/wojciech/Projects/akeero-logux/.yarn/releases/yarn-3.2.1.cjs:319:63575)
    at async /Users/wojciech/Projects/akeero-logux/.yarn/releases/yarn-3.2.1.cjs:419:2513
➀ YN0000: β”” Completed in 5s 129ms
➀ YN0000: Failed with errors in 5s 135ms
ai commented

Another way to test it just to copy-paste files from the repo to node_modules/@logux/server