[go: nahoru, domu]

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

sync /dev/stdout: The handle is invalid. #991

Open
noneymous opened this issue Aug 18, 2021 · 4 comments
Open

sync /dev/stdout: The handle is invalid. #991

noneymous opened this issue Aug 18, 2021 · 4 comments

Comments

@noneymous
Copy link

Hi,

I'm using Zap in a Windows executable. I'm trying to tee multiple cores together, which is working fine. One of them is a standard Zap console core using os.Stdout. On program termination, I'm closing down all loggers, which also includes calling .Sync(). The sync function works fine on my cores but fails on the console logger (It seems the stdout handle is already gone). I'm wondering, if there is something I can do about it...

Here is a basic code sample (only involving the console core) demonstrating the issue:

// Lock stdout
ws := zapcore.Lock(os.Stdout)

// Create the encoder
enc := zapcore.NewConsoleEncoder(zap.NewDevelopmentEncoderConfig())

// Create the core
consCore := zapcore.NewCore(enc, ws, zapcore.Level(-1))

// Tee all the cores together
// My applications uses multiple cores
tee := zapcore.NewTee(consCore)

// Set the global logger
logger := zap.New(tee).Sugar()

// Send test message
logger.Infof("Test")

// Execute last sync
defer func(){
	err := logger.Sync()
	if err != nil {
		fmt.Println(err)
	}
}()

Here is the output, .Sync() failed on this console logger:

grafik

Just to make it clear, I cannot skip calling .Sync() because I have multiple cores teeed together which need the Sync call.

@abhinav
Copy link
Collaborator
abhinav commented Nov 5, 2021

I believe the issue is that File.Sync() on a stdout that is not fed into a TTY is "not allowed".
On a macOS machine, the error from the example above is:

sync /dev/stdout: inappropriate ioctl for device

This refers to the unix error ENOTTY. I'm not sure what the Windows equivalent of that is.
At least on Unix systems, you can probably guard against this with:

// import "syscall"

err := logger.Sync()
if err != nil && !errors.Is(err, syscall.ENOTTY) {
  fmt.Println(err)
}

@noneymous
Copy link
Author

Thank you for your help! Unfortunately, it didn't work. On Windows, the error seems to be of type os.PathError

Did a spew.Dump(err) on it:

grafik

I did not dare to catch and skip such kind of error, because I might also have a file logger core, which theoretically could run into a real path error!?

Instead I hardened my code bending skills and came up with the following fix. I defined my own implementation of the WriteSyncer, simply wrapping os.Stdout (instead of directly passing it), so I can control what happens when Sync() is called by Zap (spoiler: nothing).

// WrappedWriteSyncer is a helper struct implementing zapcore.WriteSyncer to
// wrap a standard os.Stdout handle, giving control over the WriteSyncer's
// Sync() function. Sync() results in an error on Windows in combination with
// os.Stdout ("sync /dev/stdout: The handle is invalid."). WrappedWriteSyncer
// simply does nothing when Sync() is called by Zap.
type WrappedWriteSyncer struct {
	file *os.File
}

func (mws WrappedWriteSyncer) Write(p []byte) (n int, err error) {
	return mws.file.Write(p)
}
func (mws WrappedWriteSyncer) Sync() error {
	return nil
}

Using it this way:

// Prepare WriteSyncer
ws := zapcore.Lock(WrappedWriteSyncer{os.Stdout})

// Create the core
enc := zapcore.NewConsoleEncoder(zap.NewDevelopmentEncoderConfig())
core := zapcore.NewCore(enc, ws, zapcore.DebugLevel)

Additional logger cores / WriteSyncers should not be affected by this and work as usual...

@abhinav
Copy link
Collaborator
abhinav commented Dec 7, 2021

Thanks for the workaround, @noneymous! I'll leave the issue open while the workaround is still needed.
Proper Windows support is something we would like to do in the future (#1000), but it's been pretty low priority.

@Dashing-Nelson
Copy link

I have used this method to check if the error is related. I have a Mac system and I get the following error:

sync /dev/stderr: bad file descriptor

err := logger.Sync()
// NOTE: we use syscall.EBADF to check if the error is specifically related to a bad file descriptor,
// which should be the case for if the stderr is a TTY.
if err != nil && (!errors.Is(err, syscall.EBADF) && !errors.Is(err, syscall.ENOTTY)) {
panic(err)
}

EBADF = Errno(0x9)

dmad1989 added a commit to dmad1989/urlcut that referenced this issue May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants