Repository: gohugoio/hugo (language: go)

Issue:
Enable hugo server SIGINT after loading bad config
If a user starts hugo server and writes to the config file in a way
that renders it invalid, hugo server stops responding to SIGINTs,
meaning that a user needs to send a SIGKILL to the hugo process to stop
it.

*commandeer.serve needs to obtain a *hugolib.HugoSites in order to
close the Hugo environment. After handling signals/the stop channel,
*commandeer.serve calls c.hugo() to get the *hugolib.HugoSites. This
function receives from the c.created channel, which is closed when the
HugoSites is ready. However, if an error took place while loading the
config, c.created would never be closed, so Hugo would block on this
channel even after handling SIGINT.

The solution is to close c.created if we failed to load a config when
rebuilding the site, then check at the end of *commandeer.serve whether
the *hugolib.HugoSites is nil. If it is, we stop waiting for a HugoSites
to close, and exit the hugo process.

One issue that resulted from this fix was how to reset the c.created
channel during site rebuilds. In the current code, this is done by
assigning c.commandeerHugoState to a newCommandeerHugoState in
*commandeer.fullRebuild. However, this creates a race condition, since
other goroutines can be reading the value of c.created just as
*commandeer.fullRebuild is attempting to reassign it.

This change fixes the race condition by adding the lazy.Notifier type,
which preserves the use of c.created while allowing for it to be reset
in a goroutine-safe way. c.created is now a lazy.Notifier. I added this
type to the lazy package because it didn't seem worth it to add a new
package, but we can do this if it's cleaner. (I was thinking about using
sync.Cond for this, but the approach I used is closer to the original
use of the c.created channel.)

Also adds an integration test for interrupting the server after fixing
an invalid configuration. An earlier version of this change caused hugo
server to stop exiting with a SIGINT after fixing a bad config, so I
wanted to ensure that we can prevent this.

Fixes #8340


Discussion / hints:
Thanks for this; I'm sure what you have implemented works, but looking at your description I think we need to take a step back and solve this by reducing some of the complexity (which I'm certainly to blame for, but I got some of it from the starting point which was lots of globals) not add more complexity.

I'm not sure what that would be, but I'll think about it. I'll leave this PR open for if I can not come up with something simpler.
👍 I'll keep this rebased in the meantime just in case.

Fix patch:
diff --git a/commands/commandeer.go b/commands/commandeer.go
index b302cbfe0a3..444d7598711 100644
--- a/commands/commandeer.go
+++ b/commands/commandeer.go
@@ -130,6 +130,15 @@ func (c *commandeerHugoState) hugo() *hugolib.HugoSites {
 	return c.hugoSites
 }
 
+func (c *commandeerHugoState) hugoTry() *hugolib.HugoSites {
+	select {
+	case <-c.created:
+		return c.hugoSites
+	case <-time.After(time.Millisecond * 100):
+		return nil
+	}
+}
+
 func (c *commandeer) errCount() int {
 	return int(c.logger.LogCounters().ErrorCounter.Count())
 }
diff --git a/commands/server.go b/commands/server.go
index f616810039c..a339a176013 100644
--- a/commands/server.go
+++ b/commands/server.go
@@ -522,18 +522,20 @@ func (c *commandeer) serve(s *serverCmd) error {
 		roots = []string{""}
 	}
 
+	templHandler := c.hugo().Tmpl()
+	errTempl, found := templHandler.Lookup("_server/error.html")
+	if !found {
+		panic("template server/error.html not found")
+	}
+
 	srv := &fileServer{
 		baseURLs: baseURLs,
 		roots:    roots,
 		c:        c,
 		s:        s,
 		errorTemplate: func(ctx any) (io.Reader, error) {
-			templ, found := c.hugo().Tmpl().Lookup("_server/error.html")
-			if !found {
-				panic("template server/error.html not found")
-			}
 			b := &bytes.Buffer{}
-			err := c.hugo().Tmpl().Execute(templ, b, ctx)
+			err := templHandler.Execute(errTempl, b, ctx)
 			return b, err
 		},
 	}
@@ -579,16 +581,37 @@ func (c *commandeer) serve(s *serverCmd) error {
 
 	jww.FEEDBACK.Println("Press Ctrl+C to stop")
 
-	if s.stop != nil {
-		select {
-		case <-sigs:
-		case <-s.stop:
+	err := func() error {
+		if s.stop != nil {
+			for {
+				select {
+				case <-sigs:
+					return nil
+				case <-s.stop:
+					return nil
+				case <-ctx.Done():
+					return ctx.Err()
+				}
+			}
+		} else {
+			for {
+				select {
+				case <-sigs:
+					return nil
+				case <-ctx.Done():
+					return ctx.Err()
+				}
+			}
 		}
-	} else {
-		<-sigs
+	}()
+
+	if err != nil {
+		jww.ERROR.Println("Error:", err)
 	}
 
-	c.hugo().Close()
+	if h := c.hugoTry(); h != nil {
+		h.Close()
+	}
 
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	defer cancel()
diff --git a/config/configLoader.go b/config/configLoader.go
index d25546cdbc7..6722c12fd73 100644
--- a/config/configLoader.go
+++ b/config/configLoader.go
@@ -59,6 +59,13 @@ func FromConfigString(config, configType string) (Provider, error) {
 func FromFile(fs afero.Fs, filename string) (Provider, error) {
 	m, err := loadConfigFromFile(fs, filename)
 	if err != nil {
+		fe := herrors.UnwrapFileError(err)
+		if fe != nil {
+			pos := fe.Position()
+			pos.Filename = filename
+			fe.UpdatePosition(pos)
+			return nil, err
+		}
 		return nil, herrors.NewFileErrorFromFile(err, filename, fs, nil)
 	}
 	return NewFrom(m), nil
diff --git a/hugolib/config.go b/hugolib/config.go
index e63d6da4ec1..ef23086b533 100644
--- a/hugolib/config.go
+++ b/hugolib/config.go
@@ -75,7 +75,7 @@ func LoadConfig(d ConfigSourceDescriptor, doWithConfig ...func(cfg config.Provid
 		if err == nil {
 			configFiles = append(configFiles, filename)
 		} else if err != ErrNoConfigFile {
-			return nil, nil, err
+			return nil, nil, l.wrapFileError(err, filename)
 		}
 	}
 
@@ -463,7 +463,7 @@ func (l configLoader) loadConfig(configName string) (string, error) {
 
 	m, err := config.FromFileToMap(l.Fs, filename)
 	if err != nil {
-		return "", l.wrapFileError(err, filename)
+		return filename, err
 	}
 
 	// Set overwrites keys of the same name, recursively.
@@ -511,5 +511,12 @@ func (configLoader) loadSiteConfig(cfg config.Provider) (scfg SiteConfig, err er
 }
 
 func (l configLoader) wrapFileError(err error, filename string) error {
+	fe := herrors.UnwrapFileError(err)
+	if fe != nil {
+		pos := fe.Position()
+		pos.Filename = filename
+		fe.UpdatePosition(pos)
+		return err
+	}
 	return herrors.NewFileErrorFromFile(err, filename, l.Fs, nil)
 }


Test patch: