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

errors: no obvious way to map error to errno values #29054

Open
jech opened this issue Dec 1, 2018 · 9 comments
Open

errors: no obvious way to map error to errno values #29054

jech opened this issue Dec 1, 2018 · 9 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@jech
Copy link

jech commented Dec 1, 2018

go version go1.11.2 linux/amd64

I'm writing some Go code that is called by C code, and I find there's no obvious way to map an error value to a Unix errno value. For example, at one point I write:

        n, err := conn.WriteToUDP(data, &addr)
	if err != nil {
		e := unix.EIO
		if os.IsTimeout(err) {
			e = unix.EAGAIN
		}
                C.set_errno(e)
                return -1
       }

While that keeps the code working, it obviously loses a lot of the error granularity. Given that the errno value must be available somewhere under the hood, it's infuriating that it's not exported.

(I'd envision for example an interface with a single method ToOsInteger and that is implemented for those errors for which it makes sense to map to a Unix errno.)

@odeke-em
Copy link
Member

odeke-em commented Dec 1, 2018

Thank you for filing this issue @jech!

Perhaps this might just be a docs discovery problem but I think that this can easily be translated today because:

a) There are syscall.Errno enumerations of values https://golang.org/pkg/syscall/#Errors
b) for functions that invoke syscall such as in net, we wrap those syscall errors into os.SyscallError https://golang.org/pkg/os/#SyscallError which you can then type switch

Despite it not being encouraged to fiddle around with the syscall package, I'll offer some tips

A suggested remedy

a) Given an error, please check if it is of type syscall.Errno or os.SyscallError
b) if it is of type syscall.Errno, your work is done and you can do your C.set_errno(e)
c) otherwise, please type assert on syscerr.Err.(syscall.Errno) and then C.set_errno(e)

which for your code snippet translates then to the following:

n, err := conn.WriteToUDP(data, &addr)
if err == nil {
     return n // or whatever value you meant to return
}

// Otherwise now translate the error to a C errno value.
var e syscall.Errno = unix.EIO

if os.IsTimeout(err) {
	e = unix.EAGAIN
} else if osscerr, ok := err.(*os.SyscallError); ok {
        e = osscerr.Err.(syscall.Errno)
} else if scerr, ok := err.(syscall.Errno); ok {
        e = scerr
}

C.set_errno(e)
return -1

@jech in the future, it would also be helpful to first discuss this on golang-dev or golang-nuts.

I'll also kindly page @ianlancetaylor @rsc, please feel free to chime in and correct me.

@jech
Copy link
Author

jech commented Dec 1, 2018

Thanks a lot for your code, that seems to do the trick. Still, would you agree that this is not something you'd expect the average Go programmer to discover on his own? And that this should either be encapsulated in a standard library function, or at least documented?

@odeke-em
Copy link
Member

odeke-em commented Dec 2, 2018

Still, would you agree that this is not something you'd expect the average Go programmer to discover on his own?

Fair point that it would require a little bit more digging than usual, which is why I mentioned this perhaps being a docs discovery issue.

Adding it to the standard library is something to consider carefully because how many people have encountered this issue? It is a cross-package utilitiy i.e. os, syscall, it involves error introspection. But also adding it in means that its signature has to be kept intact. for the lifetime of a GoX series which can be decades
I think that what we can do is, please distill this issue into an "Experience Report" https://github.com/golang/go/wiki/ExperienceReports which will help a lot with informing the design process as we proceed towards Go2.
The "Experience Report" could be a blogpost or Github Gist and then add it to that linked listing, share it with your friends, gather feedback, and the Go maintainers will take a look. Such feedback is highly valuable and will also help with errors redesign (/cc @mpvl)
Concurrently with the report, you can create a small library that does this translation and once we have a tangible reference that's used with various applications but also demonstrates demand, that makes it really easy for decisions into including into the standard library.

@jech
Copy link
Author

jech commented Dec 2, 2018

Adding it to the standard library is something to consider carefully

That makes perfect sense. (Please take it literally -- I'm not sure whether I agree, but your argument definitely makes sense.)

(I'm not quite sure whether to close this issue — on the one hand, you've solved my immediate technical problem, for which thanks; on the other hand, I still think there's a documentation problem somewhere.)

@odeke-em
Copy link
Member

odeke-em commented Dec 2, 2018

(I'm not quite sure whether to close this issue — on the one hand, you've solved my immediate technical problem, for which thanks; on the other hand, I still think there's a documentation problem somewhere.)

Given that this is an issue that isn't frequently reported and that we have solved it, let's close it. However, as I had suggested in #29054 (comment) whenever you get some free time, please write an "Experience Report" and share it on the wiki, on golang-nuts, social media etc. In this issue I've also paged some interested parties and as we work towards Go2, the feedback will be considered towards the next design.

@networkimprov
Copy link

networkimprov commented Dec 2, 2018

I don't see how a SysErr() or Errno() method fits in the scope of either Go 2 Error Values or Error Handling, as it's specific to errors stemming from syscalls.
https://go.googlesource.com/proposal/+/master/design/go2draft.md

It seems like a method of net.Error, os.PathError, and similar types, or a new interface type. Also the solution above looks like it could be brittle.

I'd suggest leaving it open, with title "proposal: Go2: add SysErr interface type" and tag Go2. That's the common approach to gaps in the stdlib.

@ianlancetaylor ianlancetaylor changed the title No obvious way to map error to errno values errors: no obvious way to map error to errno values Dec 3, 2018
@ianlancetaylor
Copy link
Contributor

I think this issue is reasonably addressed by error.As in the current error design draft (see https://go.googlesource.com/proposal/+/master/design/go2draft.md).

@networkimprov
Copy link

Ah this should work, tho it does push you into the syscall API.

if e, ok := errors.As(syscall.Errno)(err); ok {
   C.set_errno(e)
}

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 19, 2018
@bcmills bcmills added this to the Unplanned milestone Dec 19, 2018
@tv42
Copy link

tv42 commented May 18, 2019

@networkimprov: Wanting a C errno means you want platform-specific details which means you're dealing with the syscall package, that sounds very reasonable. Consider lack of Errno in https://golang.org/pkg/syscall/?GOOS=plan9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

6 participants