CrabDude / trycatch

An asynchronous domain-based exception handler with long stack traces for node.js
MIT License
246 stars 17 forks source link

Using "process.stdout.isTTY" inside Windows service causes exception. #35

Closed ghost closed 10 years ago

ghost commented 10 years ago

When node starts from Windows service, accessing property "process.stdout.isTTY" in "lib/formatError.js" causes the following exception "Implement me. Unknown stream file type!".

In my case moving "try ... catch" block outside the block "if (process.stdout.isTTY)" resolved issue:

try {
  if (process.stdout.isTTY) {
    colors = require('colors');
  }
} catch(err) {}
rlidwka commented 10 years ago

sounds like node.js core error

ghost commented 10 years ago

It is rather a Windows API feature: function GetFileType() gives FILE_TYPE_UNKNOWN for stdout when application started from service. And then uv_guess_handle() in node.js returns UV_UNKNOWN_HANDLE for this WINAPI code.

rlidwka commented 10 years ago

sounds like libuv error :)

ghost commented 10 years ago

I'll try to report this in libuv then. Especially because this behaviour affects some other places in the service as well.

Though, since mentioned exception throws already in require('trycatch') (even before any code), I'd think that it would be useful to catch it here as well. When type of file is unknown, I think it is safe to guess that file is not a TTY.

ghost commented 10 years ago

After some more reviewing I decided, that every one function in the call chain working correctly. Windows service does not attaches console to the application, so Windows API function GetStdHandle(STD_OUTPUT_HANDLE) returns NULL value as file handle. Then GetFileType() obviously returns FILE_TYPE_UNKNOWN and so on.

Throwing an exception in this situation seems appropriate. So it is user application responsiblity to handle it (as it seems to me).

CrabDude commented 10 years ago

I don't see it documented anywhere, but it does seem to be by design that stdout may not exist. (In that it's not guaranteed, not that it's not at error at some level) Either way, coloring is a non-critical code path so trycatch should account for this.

Related: https://github.com/CrabDude/trycatch/issues/36

CrabDude commented 10 years ago

This is now a chalk (https://github.com/sindresorhus/chalk/issues/15) and has-color (https://github.com/sindresorhus/has-color/issues/5) issue after switching to them.

If they take too long, I'll fork and fix it, but in the meantime color is an optionalDependency now (https://github.com/CrabDude/trycatch/commit/7f395a1ffdeac4d073290193f0a83a15aeb169e3), so you can just remove it as a temporary workaround.

CrabDude commented 10 years ago

Fixed after deciding to stick with colors: https://github.com/CrabDude/trycatch/commit/957a11ba163e02cded22ed80afbfcdc39c6d43c5