Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ package process
import (
"context"
"errors"
"fmt"
"os"
"sync"
"time"

coreerr "forge.lthn.ai/core/go-log"
)

// DaemonOptions configures daemon mode execution.
Expand Down Expand Up @@ -72,7 +73,7 @@ func (d *Daemon) Start() error {
defer d.mu.Unlock()

if d.running {
return errors.New("daemon already running")
return coreerr.E("Daemon.Start", "daemon already running", nil)
}

if d.pid != nil {
Expand Down Expand Up @@ -100,7 +101,7 @@ func (d *Daemon) Start() error {
entry.Health = d.health.Addr()
}
if err := d.opts.Registry.Register(entry); err != nil {
return fmt.Errorf("registry: %w", err)
return coreerr.E("Daemon.Start", "registry", err)
}
Comment on lines 103 to 105
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Rollback daemon state when registry registration fails.

At Line 104, Start() returns after d.running is already true and startup resources may already be acquired. This can leave PID/health resources active after a failed start.

Suggested fix
 		if err := d.opts.Registry.Register(entry); err != nil {
+			if d.health != nil {
+				_ = d.health.Stop(context.Background())
+			}
+			if d.pid != nil {
+				_ = d.pid.Release()
+			}
+			d.running = false
 			return coreerr.E("Daemon.Start", "registry", err)
 		}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if err := d.opts.Registry.Register(entry); err != nil {
return fmt.Errorf("registry: %w", err)
return coreerr.E("Daemon.Start", "registry", err)
}
if err := d.opts.Registry.Register(entry); err != nil {
if d.health != nil {
_ = d.health.Stop(context.Background())
}
if d.pid != nil {
_ = d.pid.Release()
}
d.running = false
return coreerr.E("Daemon.Start", "registry", err)
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@daemon.go` around lines 103 - 105, Start() currently sets d.running true and
may acquire PID/health resources before calling d.opts.Registry.Register(entry);
if Register fails you must rollback those startup resources and reset d.running.
Add a deferred/cleanup path in Start (e.g. a rollback closure captured before
setting d.running) that on any early return sets d.running = false and releases
any acquired resources: remove PID file, deregister health checks, and call
d.opts.Registry.Unregister(entry) if registration succeeded; invoke that
rollback when d.opts.Registry.Register(entry) returns an error so the daemon
leaves no leftover PID/health state on failed start.

}

Expand All @@ -112,7 +113,7 @@ func (d *Daemon) Run(ctx context.Context) error {
d.mu.Lock()
if !d.running {
d.mu.Unlock()
return errors.New("daemon not started - call Start() first")
return coreerr.E("Daemon.Run", "daemon not started - call Start() first", nil)
}
d.mu.Unlock()

Expand All @@ -138,13 +139,13 @@ func (d *Daemon) Stop() error {
if d.health != nil {
d.health.SetReady(false)
if err := d.health.Stop(shutdownCtx); err != nil {
errs = append(errs, fmt.Errorf("health server: %w", err))
errs = append(errs, coreerr.E("Daemon.Stop", "health server", err))
}
}

if d.pid != nil {
if err := d.pid.Release(); err != nil && !os.IsNotExist(err) {
errs = append(errs, fmt.Errorf("pid file: %w", err))
errs = append(errs, coreerr.E("Daemon.Stop", "pid file", err))
}
}

Expand Down
8 changes: 5 additions & 3 deletions exec/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"os"
"os/exec"
"strings"

coreerr "forge.lthn.ai/core/go-log"
)

// Options configuration for command execution
Expand Down Expand Up @@ -147,17 +149,17 @@ func RunQuiet(ctx context.Context, name string, args ...string) error {
cmd := Command(ctx, name, args...).WithStderr(&stderr)
if err := cmd.Run(); err != nil {
// Include stderr in error message
return fmt.Errorf("%w: %s", err, strings.TrimSpace(stderr.String()))
return coreerr.E("RunQuiet", strings.TrimSpace(stderr.String()), err)
}
return nil
}

func wrapError(err error, name string, args []string) error {
cmdStr := name + " " + strings.Join(args, " ")
if exitErr, ok := err.(*exec.ExitError); ok {
return fmt.Errorf("command %q failed with exit code %d: %w", cmdStr, exitErr.ExitCode(), err)
return coreerr.E("wrapError", fmt.Sprintf("command %q failed with exit code %d", cmdStr, exitErr.ExitCode()), err)
}
return fmt.Errorf("failed to execute %q: %w", cmdStr, err)
return coreerr.E("wrapError", fmt.Sprintf("failed to execute %q", cmdStr), err)
}

func (c *Cmd) getLogger() Logger {
Expand Down
7 changes: 3 additions & 4 deletions global_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,9 @@ func TestGlobal_SetDefault(t *testing.T) {
assert.Equal(t, svc, Default())
})

t.Run("panics on nil", func(t *testing.T) {
assert.Panics(t, func() {
SetDefault(nil)
})
t.Run("errors on nil", func(t *testing.T) {
err := SetDefault(nil)
assert.Error(t, err)
})
}

Expand Down
64 changes: 33 additions & 31 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,21 @@ module forge.lthn.ai/core/go-process
go 1.26.0

require (
forge.lthn.ai/core/api v0.1.0
forge.lthn.ai/core/go v0.3.0
forge.lthn.ai/core/go-ws v0.1.3
forge.lthn.ai/core/api v0.1.3
forge.lthn.ai/core/go v0.3.1
forge.lthn.ai/core/go-io v0.1.5
forge.lthn.ai/core/go-log v0.0.4
forge.lthn.ai/core/go-ws v0.2.2
github.com/gin-gonic/gin v1.12.0
github.com/stretchr/testify v1.11.1
)

require (
github.com/99designs/gqlgen v0.17.87 // indirect
github.com/99designs/gqlgen v0.17.88 // indirect
github.com/KyleBanks/depth v1.2.1 // indirect
github.com/agnivade/levenshtein v1.2.1 // indirect
github.com/andybalholm/brotli v1.2.0 // indirect
github.com/bmatcuk/doublestar/v4 v4.9.1 // indirect
github.com/bmatcuk/doublestar/v4 v4.10.0 // indirect
github.com/bytedance/gopkg v0.1.3 // indirect
github.com/bytedance/sonic v1.15.0 // indirect
github.com/bytedance/sonic/loader v0.5.0 // indirect
Expand Down Expand Up @@ -43,21 +45,21 @@ require (
github.com/go-jose/go-jose/v4 v4.1.3 // indirect
github.com/go-logr/logr v1.4.3 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
github.com/go-openapi/jsonpointer v0.22.4 // indirect
github.com/go-openapi/jsonreference v0.21.2 // indirect
github.com/go-openapi/spec v0.22.0 // indirect
github.com/go-openapi/swag/conv v0.25.1 // indirect
github.com/go-openapi/swag/jsonname v0.25.4 // indirect
github.com/go-openapi/swag/jsonutils v0.25.1 // indirect
github.com/go-openapi/swag/loading v0.25.1 // indirect
github.com/go-openapi/swag/stringutils v0.25.1 // indirect
github.com/go-openapi/swag/typeutils v0.25.1 // indirect
github.com/go-openapi/swag/yamlutils v0.25.1 // indirect
github.com/go-openapi/jsonpointer v0.22.5 // indirect
github.com/go-openapi/jsonreference v0.21.5 // indirect
github.com/go-openapi/spec v0.22.4 // indirect
github.com/go-openapi/swag/conv v0.25.5 // indirect
github.com/go-openapi/swag/jsonname v0.25.5 // indirect
github.com/go-openapi/swag/jsonutils v0.25.5 // indirect
github.com/go-openapi/swag/loading v0.25.5 // indirect
github.com/go-openapi/swag/stringutils v0.25.5 // indirect
github.com/go-openapi/swag/typeutils v0.25.5 // indirect
github.com/go-openapi/swag/yamlutils v0.25.5 // indirect
github.com/go-playground/locales v0.14.1 // indirect
github.com/go-playground/universal-translator v0.18.1 // indirect
github.com/go-playground/validator/v10 v10.30.1 // indirect
github.com/go-viper/mapstructure/v2 v2.5.0 // indirect
github.com/goccy/go-json v0.10.5 // indirect
github.com/goccy/go-json v0.10.6 // indirect
github.com/goccy/go-yaml v1.19.2 // indirect
github.com/google/uuid v1.6.0 // indirect
github.com/gorilla/context v1.1.2 // indirect
Expand All @@ -76,7 +78,7 @@ require (
github.com/quic-go/qpack v0.6.0 // indirect
github.com/quic-go/quic-go v0.59.0 // indirect
github.com/redis/go-redis/v9 v9.18.0 // indirect
github.com/sosodev/duration v1.3.1 // indirect
github.com/sosodev/duration v1.4.0 // indirect
github.com/swaggo/files v1.0.1 // indirect
github.com/swaggo/gin-swagger v1.6.1 // indirect
github.com/swaggo/swag v1.16.6 // indirect
Expand All @@ -85,22 +87,22 @@ require (
github.com/vektah/gqlparser/v2 v2.5.32 // indirect
go.mongodb.org/mongo-driver/v2 v2.5.0 // indirect
go.opentelemetry.io/auto/sdk v1.2.1 // indirect
go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin v0.65.0 // indirect
go.opentelemetry.io/otel v1.40.0 // indirect
go.opentelemetry.io/otel/metric v1.40.0 // indirect
go.opentelemetry.io/otel/sdk v1.40.0 // indirect
go.opentelemetry.io/otel/trace v1.40.0 // indirect
go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin v0.67.0 // indirect
go.opentelemetry.io/otel v1.42.0 // indirect
go.opentelemetry.io/otel/metric v1.42.0 // indirect
go.opentelemetry.io/otel/sdk v1.42.0 // indirect
go.opentelemetry.io/otel/trace v1.42.0 // indirect
go.uber.org/atomic v1.11.0 // indirect
go.yaml.in/yaml/v3 v3.0.4 // indirect
golang.org/x/arch v0.23.0 // indirect
golang.org/x/crypto v0.48.0 // indirect
golang.org/x/mod v0.33.0 // indirect
golang.org/x/net v0.51.0 // indirect
golang.org/x/oauth2 v0.35.0 // indirect
golang.org/x/sync v0.19.0 // indirect
golang.org/x/sys v0.41.0 // indirect
golang.org/x/text v0.34.0 // indirect
golang.org/x/tools v0.42.0 // indirect
golang.org/x/arch v0.25.0 // indirect
golang.org/x/crypto v0.49.0 // indirect
golang.org/x/mod v0.34.0 // indirect
golang.org/x/net v0.52.0 // indirect
golang.org/x/oauth2 v0.36.0 // indirect
golang.org/x/sync v0.20.0 // indirect
golang.org/x/sys v0.42.0 // indirect
golang.org/x/text v0.35.0 // indirect
golang.org/x/tools v0.43.0 // indirect
google.golang.org/protobuf v1.36.11 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
Loading