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

Console transport does not write to debug console #1544

Closed
1 of 2 tasks
sirockin opened this issue Dec 6, 2018 · 26 comments · May be fixed by #1835
Closed
1 of 2 tasks

Console transport does not write to debug console #1544

sirockin opened this issue Dec 6, 2018 · 26 comments · May be fixed by #1835
Labels
Bug Help Wanted Additional help desired from the community Important

Comments

@sirockin
Copy link

sirockin commented Dec 6, 2018

Please tell us about your environment:

  • winston version?
    • winston@2
    • winston@3
  • _node -v outputs: v10.1.0
  • Operating System? Windows
  • Language? TypeScript

What is the problem?

When I use the built in Console transport, it will write to console when running normally (eg node ./dist/index.js ) but not to the debug console when I debug from Visual Studio Code. Standard calls to console.log()/warn()/error() do write to the debug console.

What do you expect to happen instead?

I expect all messages to write to the debug console as they would to the standard console when running normally.

Other information

The problem seems to be the use of _stderr.write() and _stdout.write() in the log() method of the transport implementation. If I replace the condition in the if statements at 49, 62 and 77 with false so that the standard console.log()/warn()/error() functions get called, the output does reach the console output. Obviously an undesirable side-effect is that the custom eol gets ignored.

@DABH
Copy link
Contributor

DABH commented Dec 27, 2018

Feels like more of a vscode bug -- why does their debug console have props like _stdout if we can't successfully write to it like that? Of course, you could have a modified transport like your workaround that works for vscode, but that may not be ideal. If you think this is something that needs to be fixed in winston feel free to offer any ideas :) But I don't think winston should check whether it's running in vscode, rather it seems like vscode's console should behave better.

@adi7ya
Copy link

adi7ya commented Dec 31, 2018

@sirockin Are you developing a vscode extension? I came across this similar issue in vsode. Anyways, you can override the log method for console transport to make it work.

@indexzero
Copy link
Member

We should address this, but needs more investigation.

@indexzero indexzero added Bug Important Help Wanted Additional help desired from the community labels Jan 26, 2019
@victorandree
Copy link

victorandree commented Mar 12, 2019

I just ran into this issue and wanted to add my very simple idea: Just use console.log, console.warn and console.error 😄

Right now, Winston is checking for console._stderr and console._stdout, preferring to write to those streams directly, but falling back to using console.warn, console.error and console.log if not available. These properties (_stderr and _stdout) are available on the global console instance in node.js, however, they're not documented as being part of the console API nor are they enumerable.

What's the upside to using the streams directly in Winston over calling the methods? To me, console in a Javascript context means specifically the console object, not "console" in general or as a proxy for streams. Wouldn't the methods always work? Especially since there is a built-in stream transport in Winston, I would interpret "console" as appropriate for basically local development.

VS Code does have the corresponding properties (probably because it's running on node), but its default "capture" for the debug console is "console" -- which probably means they've installed hooks specifically on the console.log calls, not on the streams themselves. I.e. VS Code is generating output, but not capturing it. It's probably related to this file

As others have mentioned, you can change which output VS Code captures, or change what the "console" transport does in Winston.

{
  "version": "0.2.0",
  "configurations": [
    {
      "type": "node",
      "request": "launch",
      "name": "Launch Program",
      "program": "${workspaceFolder}/index.js",
      // Capture "std" instead of "console"
      "outputCapture": "std"
    }
  ]
}

Note, if you follow the linked advice from @adi7ya, VS Code will output the line where console.log got called -- which is not where you called Winston, but the line in your overridden log. So, it might be only marginally useful...

@DABH
Copy link
Contributor

DABH commented Mar 24, 2019

Checking with VS Code folks to see whether we can make the necessary vsc settings for console transport output capturing the defaults: microsoft/vscode#69959 . Otherwise, the necessary vsc setting changes are linked from my comment there. ^^It's a valid question though, for the console transport, if a bunch of popular editors won't do the "right" thing by default, then maybe we should just change the behavior of the console transport cc @indexzero .

@melbourne2991
Copy link

melbourne2991 commented Jun 6, 2019

This also bit me, could not agree more with @victorandree.

I'm using this very barebones transport for my own purposes at the moment if anyone else finds it useful:

import winston from "winston";
import Transport from "winston-transport";

const { MESSAGE } = require("triple-beam");
const level = process.env.LOG_LEVEL || "info";

class SimpleConsoleTransport extends Transport {
  log = (info: any, callback: any) => {
    setImmediate(() => this.emit("logged", info));

    console.log(info[MESSAGE]);

    if (callback) {
      callback();
    }
  };
}

export const logger = winston.createLogger({
  level,
  defaultMeta: {},
  transports: [new SimpleConsoleTransport()]
});

@baybal
Copy link

baybal commented Aug 13, 2020

Please check on my proposal solving this in #1836

@Jimbly
Copy link

Jimbly commented Aug 31, 2020

Also ran into this issue, and it does not appear to be VSCode-specific - if I run node --inspect and then attach to it from Chrome with chrome://inspect I see only calls to console.log/etc and not anything going to the Console transport.

@hanrea
Copy link

hanrea commented Mar 8, 2021

@Idane
Copy link

Idane commented Oct 9, 2021

I'm encountering the same issue in WebStorm, this is not a vs-code only bug.

@JoshuaCWebDeveloper
Copy link

We just ran into this issue and it took us quite a while to figure out what we did wrong. In our case, this was caused by us passing an invalid log level into Winston (i.e. debug23). When this happens, Winston will just silently fail to log anything to the console transporter.

This probably isn't the root cause of this issue; I'm just posting this here in case it helps anyone.

@einsone
Copy link

einsone commented Dec 1, 2021

change to use integratedTerminal

  "version": "0.2.0",
  "configurations": [
    {
      "type": "node",
      "request": "launch",
      "name": "Launch Program",
      "program": "${workspaceFolder}/index.js",
      "console": "integratedTerminal",
    }
  ]
}```

@jmbeach
Copy link

jmbeach commented Apr 18, 2022

I extended this a bit:

/* eslint-disable @typescript-eslint/explicit-module-boundary-types */
/* eslint-disable @typescript-eslint/no-explicit-any */
import { LEVEL } from 'triple-beam';
import Transport from 'winston-transport';

// inspired by https://github.com/winstonjs/winston/issues/1544#issuecomment-499362387
export class SimpleConsoleTransport extends Transport {
  private getLogMethod(level: string) {
    if (level === 'debug') {
      return console.debug;
    } else if (level === 'info') {
      return console.info;
    } else if (level === 'critical' || level === 'error') {
      return console.error;
    } else if (level === 'warning') {
      return console.warn;
    } else {
      return console.log;
    }
  }

  override log(info: any, callback: any): void {
    setImmediate(() => this.emit('logged', info));
    const logMethod = this.getLogMethod(info[LEVEL]);
    const date = new Date();
    if (info.logStack) {
      logMethod(
        `${date.getHours()}:${date.getMinutes()}`,
        info.message,
        info.logStack
      );
    } else {
      logMethod(`${date.getHours()}:${date.getMinutes()}`, info.message);
    }
    if (callback) {
      callback();
    }
  }
}

@jay-teli
Copy link

jay-teli commented Sep 17, 2022

"console": "integratedTerminal",

"console": "integratedTerminal" ... This did the magic for me... After spending 2 hours. Thank you.

@PierTrailslight
Copy link

"console": "integratedTerminal",

"console": "integratedTerminal" ... This did the magic for me... After spending 2 hours. Thank you.

Thanks, using the same solution!

@lynoure
Copy link

lynoure commented Jun 4, 2024

Alas, for WebStorm and other JBrains IDEs the 'integratedTerminal' workaround is not available, so it would be really nice to see a fix or a documented workaround for those not using VSC

@Jimbly
Copy link

Jimbly commented Jun 4, 2024

@lynoure The SimpleConsoleTransport above should work for all platforms, that's basically the workaround I've been using in my stack (and not using VSCode).

@DABH
Copy link
Contributor

DABH commented Jun 6, 2024

Would #1835 / #1836 help for JetBrains etc.? I haven't looked at this issue since 2019, but scrolling up through the thread, it looks like that PR / proposal could help? I'm open to other ideas that would make the built-in stuff in Winston easier for everyone to use (without compromising performance or breaking backward compatibility, ideally).

@Jimbly
Copy link

Jimbly commented Jun 6, 2024

Yeah, both of those would help for the built-in Chrome/Node debugger and presumable JetBrains. I'd favor an intentional split like #1836 proposes (which is almost what choosing to use or not use the SimpleConsoleTransport above does, if I'm remembering correctly), as the current behavior (writing to stdout, but not the console in debuggers) allows attaching a debugger to a busy production server without overloading the debugger with thousands of log messages a second.

@mohd-akram
Copy link

mohd-akram commented Jun 10, 2024

IMO the correct behavior would be to output to stderr only, while also outputting to the debug console. The reason it works with the global console is because it's wrapped to log to the inspector as well. You can do the same with a custom console that outputs to stderr only by wrapping it (see here).

@mohd-akram
Copy link

I've opened a PR with a very simple fix that should resolve this, try it out.

@Jimbly
Copy link

Jimbly commented Jun 10, 2024

I'm not a maintainer, but making an explicit split sounds better than always writing to both to me. Probably always writing to both is slightly better (at least, for a default behavior, it's what people would expect) than how it is now, however writing to both is a potentially light breaking change for anyone who attaches an Inspector to busy services (e.g. to do profiling of heavy-load production servers), as funneling large amounts of log traffic to Inspector causes it crash/disconnect in my experience (presumably can't send it over the WebSocket link as quickly as it's generating new messages).

@mohd-akram
Copy link

funneling large amounts of log traffic to Inspector causes it crash/disconnect in my experience (presumably can't send it over the WebSocket link as quickly as it's generating new messages).

Does inspector not let you disable logs? There's also a stream transport that people can use if they wish to log to stdout or stderr only.

@Jimbly
Copy link

Jimbly commented Jun 10, 2024

You can disable displaying logs, but that doesn't stop them from being sent, I think. An explicitly separate for transport (or option/behavior) for stdio and console is exactly what was proposed though, I think =). Anyway, some improvement here sounds great (might just not have to be in a patch/minor version, as it's got behavioral side effects people might need to deal with).

@neilenns
Copy link
Contributor

neilenns commented Aug 6, 2024

This was addressed with the forceConsole flag in #2276

@DABH
Copy link
Contributor

DABH commented Aug 8, 2024

Per the above, closing this since #2276 was released in winston 3.14.0.

@DABH DABH closed this as completed Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Help Wanted Additional help desired from the community Important
Projects
None yet