Support fd_sync for stdout/stderr writers via optional interface

[robherley]

As far as I know, when an arbitrary io.Writer is passed for the module's stdout/stderr, it's not possible to hook into fd_sync when it is called on the respective file descriptor.

I've extended writerFile struct to attempt to call a Sync method on the inner writer interface, if it passes the type assertion.

My use case is as follows:

1. For the module's WithStdout, I'm passing in a wrapper http.ResponseWriter.
2. When the guest calls os.Stdout.Sync() I want to invoke http.Flusher.Flush() on the guest.

As long as the writer decides to implement this simple interface:

type syncer interface {
	Sync() error
}

The host can hook into whenever the client flushes the fd.

---

Also, thanks for this amazing project, I've been meaning to dive into wazero ever since I saw Takeshi's talk at GopherCon '22 in person 😄

[ncruces]

This seems to make sense. I'm inclined to merge, @evacchi wdyt?

[mathetake]

No this implicit public API should at least go into experimental package, not in internal. I won't accept this

[ncruces]

OK. I actually think it makes some amount of sense. When you give it an *os.File it gets type asserted and Sync actually does something.

So it's already type asserted magic, just that it only works for *os.File and nothing else. The problem, I guess, is where does it end (i.e. should it call Close as well?)

Also, I'm not sure how this could be done with experimental, tbh. We'd type assert experimental/sys.File and you'd need to implement your own dummy versions of everything (with UnimplementedFile as the docs state)? Would that be OK?

If so, that gets my upvote. It's not too burdensome for clients. But it's still type asserted magic, just a little more deliberate (less accidental) from clients.

[mathetake]

the problem is assertion on this "internal interface" vs the os.File concrete, isn't it? we could break anytime we want

[ncruces]

IMO the bigger problem, which hadn't occur to me, is when you “accidentally” implement the interface.

For instance, I'm not sure if I pass a io.WriteCloser, do I want Close to be called? I may want it kept open. I could hide the Close method, but that's convoluted? Sync didn't seem problematic, but Close really made me wonder.

OTOH, if you explicitly implement experimental/sys.File, it's a fat interface, experimental to boot, you're not going to do implement it accidentally. Then I think calling those methods should be fine, and you can implement them however you want.

[robherley]

Thanks for the feedback y'all

@ncruces It would be great to implement experimentalsys.File interface for my "sync" stdout/stderr use case, but unfortunately that interface is incompatible with the writer passed to WithStdout, since it takes an io.Writer and these two receiver methods would conflict:

// io.Writer
func (w *W) Write(buf []byte) (int, error) { }

// experimentalsys.File
func (w *W) Write(buf []byte) (int, experimentalsys.Errno) { }

Unless I'm misunderstanding, I don't believe there is a way today I can pass the experimental type to be used for stdout/stderr. Which might be by design today because it is... experimental 😅

I also agree that satisfying a magic internal interface for Sync isn't the most elegant approach and I'm open to other ideas

[anuraaga]

@robherley Would it be possible to mount the response on a file in the FS instead of using stdout? I noticed it looks like you do that for input data so maybe it makes sense for output too

https://github.com/robherley/webfunc/blob/main/internal%2Fsandbox%2Fsandbox.go#L103

If it were trivial to work with stdout, then it would be nice but that interface incompatibility makes it look challenging and I don't think we have a general goal of wanting stdout to be too flexible, it's really meant to collect what would otherwise go to the terminal. FS is probably the more general abstraction you need, and otherwise we'd generally expect a host ABI I think.

[ncruces]

Ah, correct, you can't implement both interfaces. This would have to be a new (experimental) module configuration, which would be more work.

[robherley]

Looks like this will end up being a larger change than I expected, so I'll close this out. Thanks y'all