hugelgupf / p9

Idiomatic Go 9P2000.L client and server, extracted from gVisor for general use
Apache License 2.0
87 stars 19 forks source link

internal error packages should be exported (or something) #55

Closed djdv closed 1 year ago

djdv commented 2 years ago

tl;dr Need to be able to use errors.Is(clientError, linux.Errno(someVal)) in code that uses Client,


Long version:

internal and internal/linux pkgs can't be used by calling code, which makes handling errors client-side difficult, especially on non-Linux.

Consider a server and client on a Windows host, and a client-side API which wants to wrap Create.

Server-side, Walk and Create methods seem expected to return the os.ErrNotExist and os.ErrExist values, which get translated by clientFile internally via newErr/internal.ExtractErrno into linux.ENOENT and linux.EEXIST.

Client-side, whether you use Walk to check file existence before sending Create, or send Create alone; there's no easy way to distinguish which specific error you received.

The workaround I'm currently using requires copying the (protocol-level) error values into an accessible pkg, and then using the runtime to extract the underlying decimal values from Client errors for comparison. Which seems less than ideal.


Here's an example from a toy/prototype that might demonstrate this. (I can bundle this whole thing up if needed, but it's just for experimenting/exercising this library right now so it's gross)

This errors pkg contains direct copies of the internal linux.Errno type and values. goerrors is the standard errors pkg. (errors is a bad name on my part and should probably be something like perrors for protocol errors or something)

func makeMessage(dir p9.File, filename string) (p9.File, error) {
    const flags = p9.WriteOnly
    wnames := []string{filename}
    _, messageFile, err := dir.Walk(wnames)
    if err != nil {
        err = runtimeHax(err)
        if !goerrors.Is(err, errors.ENOENT) {
            return nil, err
        }
        _, dirClone, err := dir.Walk(nil)
        if err != nil {
            return nil, err
        }
        messageFile, _, _, err := dirClone.Create(filename, flags,
            p9.ModeRegular|p9.AllPermissions, p9.NoUID, p9.NoGID)
        return messageFile, err
    }

    // TODO: check mode attr isregular
    // TODO: truncate/setattr on open

    if _, _, err := messageFile.Open(flags); err != nil {
        return nil, err
    }
    return messageFile, nil
}

func runtimeHax(err error) errors.Errno {
    // XXX: This works but should basically be considered illegal.
    // type notError struct{ l, h unsafe.Pointer }
    // return *(*errors.Errno)((*notError)(unsafe.Pointer(&err)).h)

    // XXX: Better, not best. Still not safe.
    // We'll panic if `err`'s underlying type changes from `uintptr`, and/or need more complicated handling.
    return errors.Errno(reflect.ValueOf(err).Uint())
}

It'd be ideal if the client could call errors.Is(err, linux.ENOENT) without needing err = runtimeHax(err).


As an aside, there's a subtle discrepancy here which may or may not matter. Calling server methods directly will return host-native error values (os.ErrExist) while calling the method via Client return the protocol-native values (linux.EEXIST).

This makes sense and should probably be expected, but I'm mentioning it anyway since calling code may have to consider both host-native and protocol-level error values, despite wrapping the same methods. Although it may be expected to always utilize Client even when in the same process(, using some memcopy-like transport). And this is kind of out of scope of the problem.

In addition to this, server implementations can't return protocol-level error numbers because linux.Errno is not exported. ExtractErrno will check for this type and pass it through to the client if err is one, but there's no way for a server implementation to actually return one. As a result returning something like myError(0x11) always becomes linux.EIO in Client methods.

djdv commented 1 year ago

Closed by: https://github.com/hugelgupf/p9/pull/71