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

Http Transport uses JSON format options as request options #1732

Comments

@MoritzLoewenstein
Copy link
Contributor

MoritzLoewenstein commented Nov 12, 2019

Please tell us about your environment:

  • winston version?
    • winston@2
    • winston@3
  • _node -v outputs: v13.1.0
  • _Operating System? windows & linux same behaviour
  • _Language? -

What is the problem?

If I use a property in a JSON formatted log which has the same name as an option in the configuration of the Http Transport its used as an option there (See Example Code).

What do you expect to happen instead?

I do expect that the contents of <logger>.log() directly go the logging transport and do not interfere with the transport configuration.

Example Code

const winston = require("winston");
const express = require("express");

const app = express();

app.post("/a", (req, res) => {
  console.log("This is good");
});

app.post("/b", (req, res) => {
  console.log("This is bad");
});

app.listen(80, console.log("Server started"));

const logger = winston.createLogger({
  format: winston.format.json(),
  transports: [
    new winston.transports.Http({
      host: "localhost",
      path: "/a"
    })
  ]
});

logger.log({ message: "abc", level: "info", path: "/b" });

Example Code Output

Server started
This is bad
@meyer9
Copy link

meyer9 commented Jan 18, 2023

also running into this issue... seems to be something wrong here: https://github.com/winstonjs/winston/blob/master/lib/winston/transports/http.js#L182-L183

@DABH
Copy link
Contributor

DABH commented Jan 18, 2023

PR is welcomed, if you identify a bug and can include some unit tests in a PR!

@MoritzLoewenstein
Copy link
Contributor Author

I opened a PR, i dont think its perfect but it fixes the issue as far as I can tell @DABH

@DABH DABH closed this as completed in 6926648 Feb 8, 2023
@DABH
Copy link
Contributor

DABH commented Feb 8, 2023

Going to close this issue since we've merged the PR (this fix will go out in the next release, which should probably be a minor release 3.2.0). Thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment