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

[Bug]: User reports breakage with 3.14.1 #2502

Closed
DABH opened this issue Aug 13, 2024 · 11 comments · Fixed by #2503
Closed

[Bug]: User reports breakage with 3.14.1 #2502

DABH opened this issue Aug 13, 2024 · 11 comments · Fixed by #2503

Comments

@DABH
Copy link
Contributor

DABH commented Aug 13, 2024

🔎 Search Terms

3.14.1

The problem

A user has reported a breaking change in updating from 3.14.0 to 3.14.1. This change was reported privately; the core detail is

  _consoleLog = console.log.bind(console);
              ^

SyntaxError: Unexpected token =

which is a line introduced in #2498 . Raising this as a GitHub Issue for public visibility; actively working on investigating (will just revert and push 3.14.2 if needed) but putting out several fires, hopefully will have this issue resolved in <24hr.

What version of Winston presents the issue?

v3.14.1

What version of Node are you using?

Not reported

If this worked in a previous version of Winston, which was it?

v3.14.0

Minimum Working Example

No response

Additional information

No response

@DABH
Copy link
Contributor Author

DABH commented Aug 13, 2024

@neilenns Any thoughts? Do you reckon these lines added by your PR should go in the constructor, or?

@neilenns
Copy link
Contributor

neilenns commented Aug 13, 2024

@neilenns Any thoughts? Do you reckon these lines added by your PR should go in the constructor, or?

I don't understand what issue is being reported. I pulled the latest from master and ran npm run build and it builds fine. The CI build was fine too. What's supposed to be broken?

neilenns added a commit to neilenns/winston that referenced this issue Aug 13, 2024
@neilenns
Copy link
Contributor

I made a simple test app with the following:

const winston = require("winston");

const logger = winston.createLogger({
  level: "info",
  transports: [new winston.transports.Console({ forceConsole: true })],
});

logger.info("Hi!");

And verified that the version of Winston in the package-lock.json was 3.14.1:

    "node_modules/winston": {
      "version": "3.14.1",

Everything runs fine. I don't see any errors thrown when I run the test app:

node ./index.js
{"level":"info","message":"Hi!"}

I will work to validate the PR I opened since I do believe the original fix is incorrect, but I'm at a loss for how someone hit the above error and why I'm not seeing it.

@DABH
Copy link
Contributor Author

DABH commented Aug 13, 2024

I'm wondering also if prepending this. would make any difference. But yeah, I don't (yet) know the environmental details of the user who reported the issue. I will work to find out more and see what I can determine, but otherwise, yeah, I'm just guessing as to what would be the "safest" Javascript we could write 🙃

@DABH
Copy link
Contributor Author

DABH commented Aug 13, 2024

Here is a bit more of the stack (this feels safe to share and I was not told to not share...). So this is clearly Linux, and is coming in via the package ca-apm-probe (which I do see on NPM). I also see some allusions to CommonJS. I think that might have something to do with it - I never learned the details of different module loaders and things like CJS but I know enough that these things can cause some complications. It is possible this user is trying to use Winston in the browser (so would be logging to the browser console)?

Config.startConfiguration Error : SyntaxError: Unexpected token =
/opt/app-root/src/.npm-global/lib/node_modules/ca-apm-probe/node_modules/winston/lib/winston/transports/console.js:24
  _consoleLog = console.log.bind(console);
              ^

SyntaxError: Unexpected token =
    at Object.compileFunction (vm.js:406:10)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)

@neilenns
Copy link
Contributor

The original change was probably wrong, I'm just so used to TypeScript. The move into the constructor follows the pattern of all the other class properties so it should be fine.

@timdhughes
Copy link

timdhughes commented Aug 13, 2024

Thanks for addressing this! What is the ETA on a 3.14.2 release? This is affecting an application that I maintain. It is running forever with winston as a dependency.

@DABH
Copy link
Contributor Author

DABH commented Aug 13, 2024

@timdhughes can you confirm that this proposed fix fixes things for you?

My apologies, I’ve been stuck in multiple meetings simultaneously all day, I will try to take a look at this now… Ideally, we can just get a confirmation from someone that this is indeed a fix before I push another release

@timdhughes
Copy link

@timdhughes can you confirm that this proposed fix fixes things for you?

My apologies, I’ve been stuck in multiple meetings simultaneously all day, I will try to take a look at this now… Ideally, we can just get a confirmation from someone that this is indeed a fix before I push another release

@DABH Sure! I'll try it out later today.

@timdhughes
Copy link

@DABH @neilenns This change works for me:

npm install forever "winston@https://github.com/neilenns/winston.git#neilenns/issue2502"

I would strongly suggest that you verify this independently since I just came across the winston and forever packages yesterday when I was tracking down the bug in my dev environment.

@DABH DABH closed this as completed in 2458ba6 Aug 14, 2024
@DABH
Copy link
Contributor Author

DABH commented Aug 14, 2024

v3.14.2 has been published. Thanks again all!

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

Successfully merging a pull request may close this issue.

3 participants