Comments (7)
You ask a good question.
I don't think making the libvirtError struct public is the right thing, because much of what it contains isn't going to be useful to a client of the library. Even when it comes to the error codes, most of the time clients just want to know whether a call succeeded or failed. But there may be exceptions.
I'd suggest a variation on what your PR contains: keep the libvirtError struct private. With the the Error() func you added, decodeError now returns libvirtErrors, but they're opaque errors to clients of the library. Then you can add the helper functions that reason about them to go-libvirt itself, and they look would like this:
func ErrIsNotFound(e error) bool {
err, ok := e.(libvirtError)
if !ok {
return false
}
return err.Code == ErrNotFound
}
A more complete example to show what I mean: https://play.golang.org/p/o2YyZpPoSfO
One criticism you could make of the approach I'm suggesting is that it potentially forces lots of clients of go-libvirt to add their own helper methods to the library, in the name of keeping some error codes private. But if what you're looking for is helpers that classify a range of errors, like all the connection errors, then it may be that a small number of helpers is all that will ever be needed.
from go-libvirt.
Then you can add the helper functions that reason about them to go-libvirt itself, and they look would like this:
How can I convert the error to the private type outside of its package? the PR already returns just the interface error
.
Would having a libvirt.Error which looks like this:
type Error struct {
Message string
Code ErrorCode
Domain ErrorCode
}
func (e Error) Error() string {
return e.Message
}
be acceptable?
One criticism you could make of the approach I'm suggesting is that it potentially forces lots of clients of go-libvirt to add their own helper methods to the library, in the name of keeping some error codes private.
Letting people write their own error helpers sounds good to me. What you suggest is exactly what I had in mind.
from go-libvirt.
How can I convert the error to the private type outside of its package? the PR already returns just the interface error.
Right, you can't do it outside of the go-libvirt package, because the libvirtError struct is private. But you can call the ErrIsNotFound
function above, passing in the opaque error. ErrIsNotFound
has to be part of go-libvirt, not the client.
To be clearer about what I mean, this is what I'm suggesting:
To clients of go-libvirt, any errors returned are opaque errors - opaque meaning "some type that implements the error interface". Since libvirtErrors would now implement the Error() interface, they can now be returned as opaque errors too. If client code needs to classify the error, they would call a helper function like ErrIsNotFound
above. These helper functions would need to be inside go-libvirt itself, because they attempt to cast the opaque error back to a private error type. I'm also suggesting that the error codes should remain private to go-libvirt, rather than becoming part of its public API.
from go-libvirt.
I'm also suggesting that the error codes should remain private to go-libvirt, rather than becoming part of its public API.
I agree that for many people the opaque error is good enough, and having helper functions inside go-libvirt makes sense. Also the IsNotFound
would mostly meet meet my requirement.
But to be honest, not having a public error does not sound very appealing for me. As far as I know, also in the go core packages pretty much all errors are public. Examples are os.PathError
, os.LinkError
, os.SyscallError
. Still there exist helper functions for the most common use-cases like os.IsNotExist
where the interface looks like like func IsNotExist(err error) bool
, which looks like your signature suggestion. There we have the same situation like with the huge amount of libvirt error codes. There exist a lot of errno
error codes which might be interesting for some users, but most are already happy with the predefined helper functions.
from go-libvirt.
It's true that some of the modules in the standard library have public error types, but they're not very common - I don't think there's any module in the standard library whose functions all return a special error type. The vast majority of the standard library functions return simple errors.
There are exceptions though, where the caller might need more information about the error. The ones you mention from the os package all have extra strings, so they're mostly used for building better error messages for users, but there are other examples (encoding/json.MarshalerError, for example) which have other go types. What I've never seen though are error types containing error codes, either errno values or enums from another application like libvirt. You certainly could do that, but I think there are two decent reasons not to: 1) it adds a bunch of consts to the library API, of which probably 99% will never be used, and 2) client code ends up being very non-idiomatic go.
I think there are two ways to do this don't have those problems. We could define some helper functions as I was suggesting before; or we could define some public errors, like ConnectionError; NoDomainError; etc, and return those in the right circumstances.
I don't have a problem with the second solution, actually. I just don't think the libvirt error codes should be part of the public API. This is a little different from what you were suggesting above, in that go-libvirt wouldn't have a special error type returned from all calls. Instead it would return one of the public error types when the error meets certain conditions (is a connection error, or is a domain-not-found error, etc.) I don't know that any of those error types would need any additional fields other than message, but there might be exceptions. I wouldn't put error codes in them.
Then there's the question of how many error types we'd need. I don't think the number would be very large, and I wouldn't add new ones unless there was a good reason.
from go-libvirt.
What I've never seen though are error types containing error codes, either errno values or enums from another application like libvirt.
Could it be that you mix up returning an error
interface and the actual struct? See for instance https://golang.org/pkg/syscall/#Errno. It is public and it implements the error interface but you can also cast it to the actual errno value itoa(int(e))
if needed. While the standard libs always return the error interface, all error implementations in the standard lib I know are public types.
from go-libvirt.
Closed #57 for now since I don't need this library right now. Anyway I am sorry that this did not work out. Great work you guys do here 👍
from go-libvirt.
Related Issues (20)
- Make error public, to allow more fine grained control for clients HOT 4
- Disk usage shown by ConnectGetAllDomainStats is incorrect HOT 3
- Support for connection URIs HOT 6
- Tunnel via SSH? HOT 3
- Migrate Operations missing HOT 1
- Document minimum (or maximum) supported toolchain requirements
- Mentioned method Disconnected() does not exist HOT 1
- libvirttest.MockLibvirt does not implement net.Conn HOT 2
- Broken link to libvirt RPC knowledgebase HOT 1
- Inability to subscribe `metdata-change`, `device-added` and `device-removed` events HOT 5
- Support for QEMU Agent Command HOT 4
- CI occasionally times out HOT 1
- Can libvirt arm platform run X86 system? HOT 1
- Support for modular libvirt daemons HOT 4
- suggested breaking change: add context.Context support to API HOT 1
- cannot use 2147483648 (untyped int constant) as * value in constant declaration (overflows) HOT 4
- Connecting to Libvirt
- Deadlock if Stream Shutdown races with Push
- Q: What is the meaning of `NeedResults`? HOT 1
- Qemu special api
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from go-libvirt.