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

Log with pino-http middleware raises TypeError: res.on is not a function #679

Open
kohtala opened this issue May 8, 2023 · 1 comment
Open
Labels
bug Something isn't working

Comments

@kohtala
Copy link

kohtala commented May 8, 2023

Describe the bug
Other app requests get logged with pino-http. Since the express middleware support was added in commit 24786e7, I expected to be able to log using the pino-http. But it crashes on upgrade.

To Reproduce

Engine.IO server version: 6.4.2
pino-http 8.3.3

Server

const engine = require("engine.io");
const logger = require("pino-http")();
const server = engine.listen(3000, {});

server.use(logger);
server.on("connection", (socket) => {
  console.log("connection");

  socket.on("message", (data) => {
    console.log("data", data);
  });

  socket.on("close", () => {
    console.log("close");
  });
});

Engine.IO client version: 6.2.2

Client

const { Socket } = require('engine.io-client');
const socket = new Socket('ws://localhost:3000');

socket.on("open", () => {
  console.log("open");

  socket.on("message", (data) => {
    console.log("data", data);
  });

  socket.on("close", () => {
    console.log("close");
  });
});

Expected behavior
I expected the upgrade request to be logged at upgrade time.

Platform:
Ubuntu 20.04.6, node v18.16.0.

Additional context

$ node server.js
connection
{"level":30,"time":1683554062833,"pid":2257406,"hostname":"ubuntu.mshome.net","req":{"id":1,"method":"GET","url":"/engine.io/?EIO=4&transport=polling&t=OVxf57J&b64=1","headers":{"user-agent":"node-XMLHttpRequest","accept":"*/*","host":"localhost:3000","connection":"close"},"remoteAddress":"::ffff:127.0.0.1","remotePort":36144},"res":{"statusCode":200,"headers":{}},"responseTime":6,"msg":"request completed"}
/home/kohtala/andritz/ava/js/node_modules/pino-http/logger.js:193
        res.on('close', onResponseComplete)
            ^

TypeError: res.on is not a function
    at loggingMiddleware (/home/kohtala/andritz/ava/js/node_modules/pino-http/logger.js:193:13)
    at Array.result (/home/kohtala/andritz/ava/js/node_modules/pino-http/logger.js:89:12)
    at apply (/home/kohtala/andritz/ava/js/node_modules/engine.io/build/server.js:182:32)
    at Server._applyMiddlewares (/home/kohtala/andritz/ava/js/node_modules/engine.io/build/server.js:194:9)
    at Server.handleUpgrade (/home/kohtala/andritz/ava/js/node_modules/engine.io/build/server.js:497:14)
    at Server.<anonymous> (/home/kohtala/andritz/ava/js/node_modules/engine.io/build/server.js:598:26)
    at Server.emit (node:events:513:28)
    at onParserExecuteCommon (node:_http_server:903:14)
    at onParserExecute (node:_http_server:797:3)

Node.js v18.16.0

I can see there is a small WebSocketResponse instead of a real ServerResponse. It does not implement events.

pino-http uses events to postpone the logging until request is complete.

@kohtala kohtala added the bug Something isn't working label May 8, 2023
@kohtala
Copy link
Author

kohtala commented May 9, 2023

I have worked around this by testing for req.on to be undefined and logging the upgrade in another way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant