From 712cfc06d7e541c6e28b4b127a4760b4cfccffa8 Mon Sep 17 00:00:00 2001 From: Ophestra Date: Sat, 30 Aug 2025 23:44:48 +0900 Subject: [PATCH] container: wrap container init start errors This helps indicate the exact origin and nature of the error. This eliminates generic WrapErr from container. Signed-off-by: Ophestra --- container/container.go | 55 ++++++++++---- container/container_test.go | 142 ++++++++++++++++++++++++++++++++---- container/dispatcher.go | 19 ++--- container/errors_test.go | 3 + container/init.go | 12 +-- container/init_test.go | 10 +-- container/msg.go | 34 --------- container/msg_test.go | 43 ----------- container/output.go | 7 -- container/output_test.go | 71 ++---------------- container/params.go | 15 ++-- container/params_test.go | 12 +-- internal/app/app.go | 2 +- internal/app/shim.go | 2 +- system/output.go | 8 +- system/output_test.go | 4 +- 16 files changed, 214 insertions(+), 225 deletions(-) diff --git a/container/container.go b/container/container.go index fa5a34e..2301ce7 100644 --- a/container/container.go +++ b/container/container.go @@ -99,6 +99,39 @@ type ( } ) +// A StartError contains additional information on a container startup failure. +type StartError struct { + // Fatal suggests whether this error should be considered fatal for the entire program. + Fatal bool + // Step refers to the part of the setup this error is returned from. + Step string + // Err is the underlying error. + Err error + // Origin is whether this error originated from the [Container.Start] method. + Origin bool + // Passthrough is whether the Error method is passed through to Err. + Passthrough bool +} + +func (e *StartError) Unwrap() error { return e.Err } +func (e *StartError) Error() string { + if e.Passthrough { + return e.Err.Error() + } + if e.Origin { + return e.Step + } + + { + var syscallError *os.SyscallError + if errors.As(e.Err, &syscallError) && syscallError != nil { + return e.Step + " " + syscallError.Error() + } + } + + return e.Step + ": " + e.Err.Error() +} + // Start starts the container init. The init process blocks until Serve is called. func (p *Container) Start() error { if p.cmd != nil { @@ -167,8 +200,7 @@ func (p *Container) Start() error { // place setup pipe before user supplied extra files, this is later restored by init if fd, e, err := Setup(&p.cmd.ExtraFiles); err != nil { - return wrapErrSuffix(err, - "cannot create shim setup pipe:") + return &StartError{true, "set up params stream", err, false, false} } else { p.setup = e p.cmd.Env = []string{setupEnv + "=" + strconv.Itoa(fd)} @@ -183,8 +215,7 @@ func (p *Container) Start() error { done <- func() error { // setup depending on per-thread state must happen here // PR_SET_NO_NEW_PRIVS: depends on per-thread state but acts on all processes created from that thread if err := SetNoNewPrivs(); err != nil { - return wrapErrSuffix(err, - "prctl(PR_SET_NO_NEW_PRIVS):") + return &StartError{true, "prctl(PR_SET_NO_NEW_PRIVS)", err, false, false} } // landlock: depends on per-thread state but acts on a process group @@ -200,28 +231,24 @@ func (p *Container) Start() error { // already covered by namespaces (pid) goto landlockOut } - return wrapErrSuffix(err, - "landlock does not appear to be enabled:") + return &StartError{false, "get landlock ABI", err, false, false} } else if abi < 6 { if p.HostAbstract { // see above comment goto landlockOut } - return msg.WrapErr(ENOSYS, - "kernel version too old for LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET") + return &StartError{false, "kernel version too old for LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET", ENOSYS, true, false} } else { msg.Verbosef("landlock abi version %d", abi) } if rulesetFd, err := rulesetAttr.Create(0); err != nil { - return wrapErrSuffix(err, - "cannot create landlock ruleset:") + return &StartError{true, "create landlock ruleset", err, false, false} } else { msg.Verbosef("enforcing landlock ruleset %s", rulesetAttr) if err = LandlockRestrictSelf(rulesetFd, 0); err != nil { _ = Close(rulesetFd) - return wrapErrSuffix(err, - "cannot enforce landlock ruleset:") + return &StartError{true, "enforce landlock ruleset", err, false, false} } if err = Close(rulesetFd); err != nil { msg.Verbosef("cannot close landlock ruleset: %v", err) @@ -234,7 +261,7 @@ func (p *Container) Start() error { msg.Verbose("starting container init") if err := p.cmd.Start(); err != nil { - return msg.WrapErr(err, err.Error()) + return &StartError{false, "start container init", err, false, true} } return nil }() @@ -257,7 +284,7 @@ func (p *Container) Serve() error { if p.Path == nil { p.cancel() - return msg.WrapErr(EINVAL, "invalid executable pathname") + return &StartError{false, "invalid executable pathname", EINVAL, true, false} } // do not transmit nil diff --git a/container/container_test.go b/container/container_test.go index 90fba87..0d408bf 100644 --- a/container/container_test.go +++ b/container/container_test.go @@ -10,6 +10,7 @@ import ( "os" "os/exec" "os/signal" + "reflect" "strconv" "strings" "syscall" @@ -26,6 +27,100 @@ import ( "hakurei.app/ldd" ) +func TestStartError(t *testing.T) { + testCases := []struct { + name string + err error + s string + is error + isF error + }{ + {"params env", &container.StartError{ + Fatal: true, + Step: "set up params stream", + Err: container.ErrReceiveEnv, + }, + "set up params stream: environment variable not set", + container.ErrReceiveEnv, syscall.EBADF}, + + {"params", &container.StartError{ + Fatal: true, + Step: "set up params stream", + Err: &os.SyscallError{Syscall: "pipe2", Err: syscall.EBADF}, + }, + "set up params stream pipe2: bad file descriptor", + syscall.EBADF, os.ErrInvalid}, + + {"PR_SET_NO_NEW_PRIVS", &container.StartError{ + Fatal: true, + Step: "prctl(PR_SET_NO_NEW_PRIVS)", + Err: syscall.EPERM, + }, + "prctl(PR_SET_NO_NEW_PRIVS): operation not permitted", + syscall.EPERM, syscall.EACCES}, + + {"landlock abi", &container.StartError{ + Step: "get landlock ABI", + Err: syscall.ENOSYS, + }, + "get landlock ABI: function not implemented", + syscall.ENOSYS, syscall.ENOEXEC}, + + {"landlock old", &container.StartError{ + Step: "kernel version too old for LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET", + Err: syscall.ENOSYS, + Origin: true, + }, + "kernel version too old for LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET", + syscall.ENOSYS, syscall.ENOSPC}, + + {"landlock create", &container.StartError{ + Fatal: true, + Step: "create landlock ruleset", + Err: syscall.EBADFD, + }, + "create landlock ruleset: file descriptor in bad state", + syscall.EBADFD, syscall.EBADF}, + + {"landlock enforce", &container.StartError{ + Fatal: true, + Step: "enforce landlock ruleset", + Err: syscall.ENOTRECOVERABLE, + }, + "enforce landlock ruleset: state not recoverable", + syscall.ENOTRECOVERABLE, syscall.ETIMEDOUT}, + + {"start", &container.StartError{ + Step: "start container init", + Err: &os.PathError{ + Op: "fork/exec", + Path: "/proc/nonexistent", + Err: syscall.ENOENT, + }, Passthrough: true, + }, + "fork/exec /proc/nonexistent: no such file or directory", + syscall.ENOENT, syscall.ENOSYS}, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Run("error", func(t *testing.T) { + if got := tc.err.Error(); got != tc.s { + t.Errorf("Error: %q, want %q", got, tc.s) + } + }) + + t.Run("is", func(t *testing.T) { + if !errors.Is(tc.err, tc.is) { + t.Error("Is: unexpected false") + } + if errors.Is(tc.err, tc.isF) { + t.Errorf("Is: unexpected true") + } + }) + }) + } +} + const ( ignore = "\x00" ignoreV = -1 @@ -217,9 +312,11 @@ func TestContainer(t *testing.T) { t.Run("cancel", testContainerCancel(nil, func(t *testing.T, c *container.Container) { wantErr := context.Canceled wantExitCode := 0 - if err := c.Wait(); !errors.Is(err, wantErr) { - container.GetOutput().PrintBaseErr(err, "wait:") - t.Errorf("Wait: error = %v, want %v", err, wantErr) + if err := c.Wait(); !reflect.DeepEqual(err, wantErr) { + if m, ok := container.InternalMessageFromError(err); ok { + t.Error(m) + } + t.Errorf("Wait: error = %#v, want %#v", err, wantErr) } if ps := c.ProcessState(); ps == nil { t.Errorf("ProcessState unexpectedly returned nil") @@ -233,7 +330,9 @@ func TestContainer(t *testing.T) { }, func(t *testing.T, c *container.Container) { var exitError *exec.ExitError if err := c.Wait(); !errors.As(err, &exitError) { - container.GetOutput().PrintBaseErr(err, "wait:") + if m, ok := container.InternalMessageFromError(err); ok { + t.Error(m) + } t.Errorf("Wait: error = %v", err) } if code := exitError.ExitCode(); code != blockExitCodeInterrupt { @@ -313,17 +412,26 @@ func TestContainer(t *testing.T) { if err := c.Start(); err != nil { _, _ = output.WriteTo(os.Stdout) - container.GetOutput().PrintBaseErr(err, "start:") - t.Fatalf("cannot start container: %v", err) + if m, ok := container.InternalMessageFromError(err); ok { + t.Fatal(m) + } else { + t.Fatalf("cannot start container: %v", err) + } } else if err = c.Serve(); err != nil { _, _ = output.WriteTo(os.Stdout) - container.GetOutput().PrintBaseErr(err, "serve:") - t.Errorf("cannot serve setup params: %v", err) + if m, ok := container.InternalMessageFromError(err); ok { + t.Error(m) + } else { + t.Errorf("cannot serve setup params: %v", err) + } } if err := c.Wait(); err != nil { _, _ = output.WriteTo(os.Stdout) - container.GetOutput().PrintBaseErr(err, "wait:") - t.Fatalf("wait: %v", err) + if m, ok := container.InternalMessageFromError(err); ok { + t.Fatal(m) + } else { + t.Fatalf("wait: %v", err) + } } }) } @@ -376,11 +484,17 @@ func testContainerCancel( } if err := c.Start(); err != nil { - container.GetOutput().PrintBaseErr(err, "start:") - t.Fatalf("cannot start container: %v", err) + if m, ok := container.InternalMessageFromError(err); ok { + t.Fatal(m) + } else { + t.Fatalf("cannot start container: %v", err) + } } else if err = c.Serve(); err != nil { - container.GetOutput().PrintBaseErr(err, "serve:") - t.Errorf("cannot serve setup params: %v", err) + if m, ok := container.InternalMessageFromError(err); ok { + t.Error(m) + } else { + t.Errorf("cannot serve setup params: %v", err) + } } <-ready cancel() diff --git a/container/dispatcher.go b/container/dispatcher.go index c94d934..dc8d9a4 100644 --- a/container/dispatcher.go +++ b/container/dispatcher.go @@ -138,8 +138,6 @@ type syscallDispatcher interface { resume() bool // beforeExit provides [Msg.BeforeExit]. beforeExit() - // printBaseErr provides [Msg.PrintBaseErr]. - printBaseErr(err error, fallback string) } // direct implements syscallDispatcher on the current kernel. @@ -234,12 +232,11 @@ func (direct) wait4(pid int, wstatus *syscall.WaitStatus, options int, rusage *s return syscall.Wait4(pid, wstatus, options, rusage) } -func (direct) printf(format string, v ...any) { log.Printf(format, v...) } -func (direct) fatal(v ...any) { log.Fatal(v...) } -func (direct) fatalf(format string, v ...any) { log.Fatalf(format, v...) } -func (direct) verbose(v ...any) { msg.Verbose(v...) } -func (direct) verbosef(format string, v ...any) { msg.Verbosef(format, v...) } -func (direct) suspend() { msg.Suspend() } -func (direct) resume() bool { return msg.Resume() } -func (direct) beforeExit() { msg.BeforeExit() } -func (direct) printBaseErr(err error, fallback string) { msg.PrintBaseErr(err, fallback) } +func (direct) printf(format string, v ...any) { log.Printf(format, v...) } +func (direct) fatal(v ...any) { log.Fatal(v...) } +func (direct) fatalf(format string, v ...any) { log.Fatalf(format, v...) } +func (direct) verbose(v ...any) { msg.Verbose(v...) } +func (direct) verbosef(format string, v ...any) { msg.Verbosef(format, v...) } +func (direct) suspend() { msg.Suspend() } +func (direct) resume() bool { return msg.Resume() } +func (direct) beforeExit() { msg.BeforeExit() } diff --git a/container/errors_test.go b/container/errors_test.go index df69c4b..45a5fcd 100644 --- a/container/errors_test.go +++ b/container/errors_test.go @@ -162,3 +162,6 @@ func TestErrnoFallback(t *testing.T) { }) } } + +// InternalMessageFromError exports messageFromError for other tests. +func InternalMessageFromError(err error) (string, bool) { return messageFromError(err) } diff --git a/container/init.go b/container/init.go index e56b4cf..c2208be 100644 --- a/container/init.go +++ b/container/init.go @@ -116,7 +116,7 @@ func initEntrypoint(k syscallDispatcher, prepareLogger func(prefix string), setV if errors.Is(err, EBADF) { k.fatal("invalid setup descriptor") } - if errors.Is(err, ErrNotSet) { + if errors.Is(err, ErrReceiveEnv) { k.fatal("HAKUREI_SETUP not set") } @@ -187,10 +187,7 @@ func initEntrypoint(k syscallDispatcher, prepareLogger func(prefix string), setV if m, ok := messageFromError(err); ok { k.fatal(m) } else { - k.printBaseErr(err, - fmt.Sprintf("cannot prepare op at index %d:", i)) - k.beforeExit() - k.exit(1) + k.fatalf("cannot prepare op at index %d: %v", i, err) } } } @@ -231,10 +228,7 @@ func initEntrypoint(k syscallDispatcher, prepareLogger func(prefix string), setV if m, ok := messageFromError(err); ok { k.fatal(m) } else { - k.printBaseErr(err, - fmt.Sprintf("cannot apply op at index %d:", i)) - k.beforeExit() - k.exit(1) + k.fatalf("cannot apply op at index %d: %v", i, err) } } } diff --git a/container/init_test.go b/container/init_test.go index bc29770..f045293 100644 --- a/container/init_test.go +++ b/container/init_test.go @@ -51,7 +51,7 @@ func TestInitEntrypoint(t *testing.T) { {"lockOSThread", expectArgs{}, nil, nil}, {"getpid", expectArgs{}, 1, nil}, {"setPtracer", expectArgs{uintptr(0)}, nil, nil}, - {"receive", expectArgs{"HAKUREI_SETUP", new(initParams), new(uintptr)}, nil, ErrNotSet}, + {"receive", expectArgs{"HAKUREI_SETUP", new(initParams), new(uintptr)}, nil, ErrReceiveEnv}, {"fatal", expectArgs{[]any{"HAKUREI_SETUP not set"}}, nil, nil}, }, }, nil}, @@ -410,9 +410,7 @@ func TestInitEntrypoint(t *testing.T) { {"mount", expectArgs{"", "/", "", uintptr(0x8c000), ""}, nil, nil}, /* begin early */ {"evalSymlinks", expectArgs{"/"}, "/", errUnique}, - {"printBaseErr", expectArgs{errUnique, "cannot prepare op at index 0:"}, nil, nil}, - {"beforeExit", expectArgs{}, nil, nil}, - {"exit", expectArgs{1}, nil, nil}, + {"fatalf", expectArgs{"cannot prepare op at index %d: %v", []any{0, errUnique}}, nil, nil}, /* end early */ }, }, nil}, @@ -798,9 +796,7 @@ func TestInitEntrypoint(t *testing.T) { {"verbosef", expectArgs{"%s %s", []any{"mounting", &MountProcOp{Target: MustAbs("/proc/")}}}, nil, nil}, {"mkdirAll", expectArgs{"/sysroot/proc", os.FileMode(0755)}, nil, nil}, {"mount", expectArgs{"proc", "/sysroot/proc", "proc", uintptr(0xe), ""}, nil, errUnique}, - {"printBaseErr", expectArgs{errUnique, "cannot apply op at index 1:"}, nil, nil}, - {"beforeExit", expectArgs{}, nil, nil}, - {"exit", expectArgs{1}, nil, nil}, + {"fatalf", expectArgs{"cannot apply op at index %d: %v", []any{1, errUnique}}, nil, nil}, /* end apply */ }, }, nil}, diff --git a/container/msg.go b/container/msg.go index f66c804..40f0bd4 100644 --- a/container/msg.go +++ b/container/msg.go @@ -1,25 +1,17 @@ package container import ( - "errors" - "fmt" "log" - "os" - "reflect" "sync/atomic" - "testing" ) type Msg interface { IsVerbose() bool Verbose(v ...any) Verbosef(format string, v ...any) - WrapErr(err error, a ...any) error - PrintBaseErr(err error, fallback string) Suspend() Resume() bool - BeforeExit() } @@ -37,32 +29,6 @@ func (msg *DefaultMsg) Verbosef(format string, v ...any) { } } -// checkedWrappedErr implements error with strict checks for wrapped values. -type checkedWrappedErr struct { - err error - a []any -} - -func (c *checkedWrappedErr) Error() string { return fmt.Sprintf("%v, a = %s", c.err, c.a) } -func (c *checkedWrappedErr) Is(err error) bool { - var concreteErr *checkedWrappedErr - if !errors.As(err, &concreteErr) { - return false - } - return reflect.DeepEqual(c, concreteErr) -} - -func (msg *DefaultMsg) WrapErr(err error, a ...any) error { - // provide a mostly bulletproof path to bypass this behaviour in tests - if testing.Testing() && os.Getenv("GOPATH") != Nonexistent { - return &checkedWrappedErr{err, a} - } - - log.Println(a...) - return err -} -func (msg *DefaultMsg) PrintBaseErr(err error, fallback string) { log.Println(fallback, err) } - func (msg *DefaultMsg) Suspend() { msg.inactive.Store(true) } func (msg *DefaultMsg) Resume() bool { return msg.inactive.CompareAndSwap(true, false) } func (msg *DefaultMsg) BeforeExit() {} diff --git a/container/msg_test.go b/container/msg_test.go index 2ad4249..3c68a50 100644 --- a/container/msg_test.go +++ b/container/msg_test.go @@ -1,21 +1,15 @@ package container_test import ( - "errors" "log" "strings" "sync/atomic" - "syscall" "testing" "hakurei.app/container" - "hakurei.app/internal/hlog" ) func TestDefaultMsg(t *testing.T) { - // bypass WrapErr testing behaviour - t.Setenv("GOPATH", container.Nonexistent) - { w := log.Writer() f := log.Flags() @@ -48,21 +42,6 @@ func TestDefaultMsg(t *testing.T) { } }) - t.Run("wrapErr", func(t *testing.T) { - buf := new(strings.Builder) - log.SetOutput(buf) - log.SetFlags(0) - if err := msg.WrapErr(syscall.EBADE, "\x00", "\x00"); err != syscall.EBADE { - t.Errorf("WrapErr: %v", err) - } - msg.PrintBaseErr(syscall.ENOTRECOVERABLE, "cannot cuddle cat:") - - want := "\x00 \x00\ncannot cuddle cat: state not recoverable\n" - if buf.String() != want { - t.Errorf("WrapErr: %q, want %q", buf.String(), want) - } - }) - t.Run("inactive", func(t *testing.T) { { inactive := msg.Resume() @@ -83,25 +62,6 @@ func TestDefaultMsg(t *testing.T) { // the function is a noop t.Run("beforeExit", func(t *testing.T) { msg.BeforeExit() }) - - t.Run("checkedWrappedErr", func(t *testing.T) { - // temporarily re-enable testing behaviour - t.Setenv("GOPATH", "") - wrappedErr := msg.WrapErr(syscall.ENOTRECOVERABLE, "cannot cuddle cat:", syscall.ENOTRECOVERABLE) - - t.Run("string", func(t *testing.T) { - want := "state not recoverable, a = [cannot cuddle cat: state not recoverable]" - if got := wrappedErr.Error(); got != want { - t.Errorf("Error: %q, want %q", got, want) - } - }) - - t.Run("bad concrete type", func(t *testing.T) { - if errors.Is(wrappedErr, syscall.ENOTRECOVERABLE) { - t.Error("incorrect type assertion") - } - }) - }) } type panicWriter struct{} @@ -139,9 +99,6 @@ func (out *testOutput) Verbosef(format string, v ...any) { out.t.Logf(format, v...) } -func (out *testOutput) WrapErr(err error, a ...any) error { return hlog.WrapErr(err, a...) } -func (out *testOutput) PrintBaseErr(err error, fallback string) { hlog.PrintBaseError(err, fallback) } - func (out *testOutput) Suspend() { if out.suspended.CompareAndSwap(false, true) { out.Verbose("suspend called") diff --git a/container/output.go b/container/output.go index 4fd9f09..50da335 100644 --- a/container/output.go +++ b/container/output.go @@ -10,10 +10,3 @@ func SetOutput(v Msg) { msg = v } } - -func wrapErrSuffix(err error, a ...any) error { - if err == nil { - return nil - } - return msg.WrapErr(err, append(a, err)...) -} diff --git a/container/output_test.go b/container/output_test.go index f6d629e..85ef756 100644 --- a/container/output_test.go +++ b/container/output_test.go @@ -1,8 +1,6 @@ package container import ( - "reflect" - "syscall" "testing" ) @@ -31,70 +29,13 @@ func TestGetSetOutput(t *testing.T) { }) } -func TestWrapErr(t *testing.T) { - { - out := GetOutput() - t.Cleanup(func() { SetOutput(out) }) - } - - var wrapFp *func(error, ...any) error - s := new(stubOutput) - SetOutput(s) - wrapFp = &s.wrapF - - testCases := []struct { - name string - f func(t *testing.T) - wantErr error - wantA []any - }{ - {"suffix nil", func(t *testing.T) { - if err := wrapErrSuffix(nil, "\x00"); err != nil { - t.Errorf("wrapErrSuffix: %v", err) - } - }, nil, nil}, - {"suffix val", func(t *testing.T) { - if err := wrapErrSuffix(syscall.ENOTRECOVERABLE, "\x00\x00"); err != syscall.ENOTRECOVERABLE { - t.Errorf("wrapErrSuffix: %v", err) - } - }, syscall.ENOTRECOVERABLE, []any{"\x00\x00", syscall.ENOTRECOVERABLE}}, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - var ( - gotErr error - gotA []any - ) - *wrapFp = func(err error, a ...any) error { gotErr = err; gotA = a; return err } - - tc.f(t) - if gotErr != tc.wantErr { - t.Errorf("WrapErr: err = %v, want %v", gotErr, tc.wantErr) - } - - if !reflect.DeepEqual(gotA, tc.wantA) { - t.Errorf("WrapErr: a = %v, want %v", gotA, tc.wantA) - } - }) - } -} - type stubOutput struct { wrapF func(error, ...any) error } -func (*stubOutput) IsVerbose() bool { panic("unreachable") } -func (*stubOutput) Verbose(...any) { panic("unreachable") } -func (*stubOutput) Verbosef(string, ...any) { panic("unreachable") } -func (*stubOutput) PrintBaseErr(error, string) { panic("unreachable") } -func (*stubOutput) Suspend() { panic("unreachable") } -func (*stubOutput) Resume() bool { panic("unreachable") } -func (*stubOutput) BeforeExit() { panic("unreachable") } - -func (s *stubOutput) WrapErr(err error, v ...any) error { - if s.wrapF == nil { - panic("unreachable") - } - return s.wrapF(err, v...) -} +func (*stubOutput) IsVerbose() bool { panic("unreachable") } +func (*stubOutput) Verbose(...any) { panic("unreachable") } +func (*stubOutput) Verbosef(string, ...any) { panic("unreachable") } +func (*stubOutput) Suspend() { panic("unreachable") } +func (*stubOutput) Resume() bool { panic("unreachable") } +func (*stubOutput) BeforeExit() { panic("unreachable") } diff --git a/container/params.go b/container/params.go index 00c98cf..f9862a1 100644 --- a/container/params.go +++ b/container/params.go @@ -8,11 +8,6 @@ import ( "syscall" ) -var ( - ErrNotSet = errors.New("environment variable not set") - ErrFdFormat = errors.New("bad file descriptor representation") -) - // Setup appends the read end of a pipe for setup params transmission and returns its fd. func Setup(extraFiles *[]*os.File) (int, *gob.Encoder, error) { if r, w, err := os.Pipe(); err != nil { @@ -24,19 +19,23 @@ func Setup(extraFiles *[]*os.File) (int, *gob.Encoder, error) { } } +var ( + ErrReceiveEnv = errors.New("environment variable not set") +) + // Receive retrieves setup fd from the environment and receives params. func Receive(key string, e any, fdp *uintptr) (func() error, error) { var setup *os.File if s, ok := os.LookupEnv(key); !ok { - return nil, ErrNotSet + return nil, ErrReceiveEnv } else { if fd, err := strconv.Atoi(s); err != nil { - return nil, ErrFdFormat + return nil, errors.Unwrap(err) } else { setup = os.NewFile(uintptr(fd), "setup") if setup == nil { - return nil, syscall.EBADF + return nil, syscall.EDOM } if fdp != nil { *fdp = setup.Fd() diff --git a/container/params_test.go b/container/params_test.go index 720f0bf..a65f686 100644 --- a/container/params_test.go +++ b/container/params_test.go @@ -29,8 +29,8 @@ func TestSetupReceive(t *testing.T) { }) } - if _, err := container.Receive(key, nil, nil); !errors.Is(err, container.ErrNotSet) { - t.Errorf("Receive: error = %v, want %v", err, container.ErrNotSet) + if _, err := container.Receive(key, nil, nil); !errors.Is(err, container.ErrReceiveEnv) { + t.Errorf("Receive: error = %v, want %v", err, container.ErrReceiveEnv) } }) @@ -38,8 +38,8 @@ func TestSetupReceive(t *testing.T) { const key = "TEST_ENV_FORMAT" t.Setenv(key, "") - if _, err := container.Receive(key, nil, nil); !errors.Is(err, container.ErrFdFormat) { - t.Errorf("Receive: error = %v, want %v", err, container.ErrFdFormat) + if _, err := container.Receive(key, nil, nil); !errors.Is(err, strconv.ErrSyntax) { + t.Errorf("Receive: error = %v, want %v", err, strconv.ErrSyntax) } }) @@ -47,8 +47,8 @@ func TestSetupReceive(t *testing.T) { const key = "TEST_ENV_RANGE" t.Setenv(key, "-1") - if _, err := container.Receive(key, nil, nil); !errors.Is(err, syscall.EBADF) { - t.Errorf("Receive: error = %v, want %v", err, syscall.EBADF) + if _, err := container.Receive(key, nil, nil); !errors.Is(err, syscall.EDOM) { + t.Errorf("Receive: error = %v, want %v", err, syscall.EDOM) } }) diff --git a/internal/app/app.go b/internal/app/app.go index 17b4103..2039213 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -62,7 +62,7 @@ func (a *App) String() string { return fmt.Sprintf("(unsealed app %s)", a.id) } -// Seal determines the outcome of [hst.Config] as a [SealedApp]. +// Seal determines the [Outcome] of [hst.Config]. // Values stored in and referred to by [hst.Config] might be overwritten and must not be used again. func (a *App) Seal(config *hst.Config) (*Outcome, error) { a.mu.Lock() diff --git a/internal/app/shim.go b/internal/app/shim.go index 33ede52..960506f 100644 --- a/internal/app/shim.go +++ b/internal/app/shim.go @@ -65,7 +65,7 @@ func ShimMain() { if errors.Is(err, syscall.EBADF) { log.Fatal("invalid config descriptor") } - if errors.Is(err, container.ErrNotSet) { + if errors.Is(err, container.ErrReceiveEnv) { log.Fatal("HAKUREI_SHIM not set") } diff --git a/system/output.go b/system/output.go index ec1802b..f3a4bc5 100644 --- a/system/output.go +++ b/system/output.go @@ -33,14 +33,16 @@ func (e *OpError) Error() string { } switch { - case errors.As(e.Err, new(*os.PathError)), errors.As(e.Err, new(*net.OpError)): + case errors.As(e.Err, new(*os.PathError)), + errors.As(e.Err, new(*net.OpError)), + errors.As(e.Err, new(*container.StartError)): return e.Err.Error() default: if !e.Revert { - return "cannot apply " + e.Op + ": " + e.Err.Error() + return "apply " + e.Op + ": " + e.Err.Error() } else { - return "cannot revert " + e.Op + ": " + e.Err.Error() + return "revert " + e.Op + ": " + e.Err.Error() } } } diff --git a/system/output_test.go b/system/output_test.go index 5ad219b..ba760e8 100644 --- a/system/output_test.go +++ b/system/output_test.go @@ -26,11 +26,11 @@ func TestOpError(t *testing.T) { ErrDBusConfig, syscall.ENOTRECOVERABLE}, {"apply", newOpError("tmpfile", syscall.EBADE, false), - "cannot apply tmpfile: invalid exchange", + "apply tmpfile: invalid exchange", syscall.EBADE, syscall.EBADF}, {"revert", newOpError("wayland", syscall.EBADF, true), - "cannot revert wayland: bad file descriptor", + "revert wayland: bad file descriptor", syscall.EBADF, syscall.EBADE}, {"path", newOpError("tmpfile", &os.PathError{Op: "stat", Path: "/run/dbus", Err: syscall.EISDIR}, false),