Skip to content

Commit

Permalink
hugolib/commands: Fix stuck server error issues
Browse files Browse the repository at this point in the history
Fixes #11378
  • Loading branch information
bep committed Oct 24, 2024
1 parent 5bbe95f commit cb6e27b
Show file tree
Hide file tree
Showing 12 changed files with 143 additions and 84 deletions.
2 changes: 1 addition & 1 deletion commands/commandeer.go
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ func (r *rootCommand) createLogger(running bool) (loggers.Logger, error) {
return loggers.New(optsLogger), nil
}

func (r *rootCommand) Reset() {
func (r *rootCommand) resetLogs() {
r.logger.Reset()
loggers.Log().Reset()
}
Expand Down
58 changes: 34 additions & 24 deletions commands/hugobuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"sync/atomic"
"time"

"github.com/bep/logg"
"github.com/bep/simplecobra"
"github.com/fsnotify/fsnotify"
"github.com/gohugoio/hugo/common/herrors"
Expand Down Expand Up @@ -136,10 +135,6 @@ func (e *hugoBuilderErrState) wasErr() bool {
return e.waserr
}

func (c *hugoBuilder) errCount() int {
return c.r.logger.LoggCount(logg.LevelError) + loggers.Log().LoggCount(logg.LevelError)
}

// getDirList provides NewWatcher() with a list of directories to watch for changes.
func (c *hugoBuilder) getDirList() ([]string, error) {
h, err := c.hugo()
Expand Down Expand Up @@ -345,7 +340,6 @@ func (c *hugoBuilder) newWatcher(pollIntervalStr string, dirList ...string) (*wa
for {
select {
case changes := <-c.r.changesFromBuild:
c.errState.setBuildErr(nil)
unlock, err := h.LockBuild()
if err != nil {
c.r.logger.Errorln("Failed to acquire a build lock: %s", err)
Expand All @@ -358,7 +352,7 @@ func (c *hugoBuilder) newWatcher(pollIntervalStr string, dirList ...string) (*wa
}
if c.s != nil && c.s.doLiveReload {
doReload := c.changeDetector == nil || len(c.changeDetector.changed()) > 0
doReload = doReload || c.showErrorInBrowser && c.errCount() > 0
doReload = doReload || c.showErrorInBrowser && c.errState.buildErr() != nil
if doReload {
livereload.ForceRefresh()
}
Expand All @@ -372,7 +366,7 @@ func (c *hugoBuilder) newWatcher(pollIntervalStr string, dirList ...string) (*wa
return
}
c.handleEvents(watcher, staticSyncer, evs, configSet)
if c.showErrorInBrowser && c.errCount() > 0 {
if c.showErrorInBrowser && c.errState.buildErr() != nil {
// Need to reload browser to show the error
livereload.ForceRefresh()
}
Expand Down Expand Up @@ -419,11 +413,17 @@ func (c *hugoBuilder) build() error {
}

func (c *hugoBuilder) buildSites(noBuildLock bool) (err error) {
h, err := c.hugo()
defer func() {
c.errState.setBuildErr(err)
}()

var h *hugolib.HugoSites
h, err = c.hugo()
if err != nil {
return err
return
}
return h.Build(hugolib.BuildCfg{NoBuildLock: noBuildLock})
err = h.Build(hugolib.BuildCfg{NoBuildLock: noBuildLock})
return
}

func (c *hugoBuilder) copyStatic() (map[string]uint64, error) {
Expand Down Expand Up @@ -619,6 +619,9 @@ func (c *hugoBuilder) fullRebuild(changeType string) {
// Set the processing on pause until the state is recovered.
c.errState.setPaused(true)
c.handleBuildErr(err, "Failed to reload config")
if c.s.doLiveReload {
livereload.ForceRefresh()
}
} else {
c.errState.setPaused(false)
}
Expand Down Expand Up @@ -1081,37 +1084,44 @@ func (c *hugoBuilder) printChangeDetected(typ string) {
c.r.logger.Println(htime.Now().Format(layout))
}

func (c *hugoBuilder) rebuildSites(events []fsnotify.Event) error {
func (c *hugoBuilder) rebuildSites(events []fsnotify.Event) (err error) {
defer func() {
c.errState.setBuildErr(err)
}()
if err := c.errState.buildErr(); err != nil {
ferrs := herrors.UnwrapFileErrorsWithErrorContext(err)
for _, err := range ferrs {
events = append(events, fsnotify.Event{Name: err.Position().Filename, Op: fsnotify.Write})
}
}
c.errState.setBuildErr(nil)
h, err := c.hugo()
var h *hugolib.HugoSites
h, err = c.hugo()
if err != nil {
return err
return
}

return h.Build(hugolib.BuildCfg{NoBuildLock: true, RecentlyVisited: c.visitedURLs, ErrRecovery: c.errState.wasErr()}, events...)
err = h.Build(hugolib.BuildCfg{NoBuildLock: true, RecentlyVisited: c.visitedURLs, ErrRecovery: c.errState.wasErr()}, events...)
return
}

func (c *hugoBuilder) rebuildSitesForChanges(ids []identity.Identity) error {
c.errState.setBuildErr(nil)
h, err := c.hugo()
func (c *hugoBuilder) rebuildSitesForChanges(ids []identity.Identity) (err error) {
defer func() {
c.errState.setBuildErr(err)
}()

var h *hugolib.HugoSites
h, err = c.hugo()
if err != nil {
return err
return
}
whatChanged := &hugolib.WhatChanged{}
whatChanged.Add(ids...)
err = h.Build(hugolib.BuildCfg{NoBuildLock: true, WhatChanged: whatChanged, RecentlyVisited: c.visitedURLs, ErrRecovery: c.errState.wasErr()})
c.errState.setBuildErr(err)
return err

return
}

func (c *hugoBuilder) reloadConfig() error {
c.r.Reset()
c.r.resetLogs()
c.r.configVersionID.Add(1)

if err := c.withConfE(func(conf *commonConfig) error {
Expand Down
20 changes: 11 additions & 9 deletions commands/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -648,9 +648,8 @@ func (c *serverCommand) setServerInfoInConfig() error {
}

func (c *serverCommand) getErrorWithContext() any {
errCount := c.errCount()

if errCount == 0 {
buildErr := c.errState.buildErr()
if buildErr == nil {
return nil
}

Expand All @@ -659,7 +658,7 @@ func (c *serverCommand) getErrorWithContext() any {
m["Error"] = cleanErrorLog(c.r.logger.Errors())

m["Version"] = hugo.BuildVersionString()
ferrors := herrors.UnwrapFileErrorsWithErrorContext(c.errState.buildErr())
ferrors := herrors.UnwrapFileErrorsWithErrorContext(buildErr)
m["Files"] = ferrors

return m
Expand Down Expand Up @@ -830,22 +829,25 @@ func (c *serverCommand) fixURL(baseURLFromConfig, baseURLFromFlag string, port i
return u.String(), nil
}

func (c *serverCommand) partialReRender(urls ...string) error {
func (c *serverCommand) partialReRender(urls ...string) (err error) {
defer func() {
c.errState.setWasErr(false)
}()
c.errState.setBuildErr(nil)
visited := types.NewEvictingStringQueue(len(urls))
for _, url := range urls {
visited.Add(url)
}

h, err := c.hugo()
var h *hugolib.HugoSites
h, err = c.hugo()
if err != nil {
return err
return
}

// Note: We do not set NoBuildLock as the file lock is not acquired at this stage.
return h.Build(hugolib.BuildCfg{NoBuildLock: false, RecentlyVisited: visited, PartialReRender: true, ErrRecovery: c.errState.wasErr()})
err = h.Build(hugolib.BuildCfg{NoBuildLock: false, RecentlyVisited: visited, PartialReRender: true, ErrRecovery: c.errState.wasErr()})

return
}

func (c *serverCommand) serve() error {
Expand Down
3 changes: 0 additions & 3 deletions hugolib/hugo_sites.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,6 @@ type hugoSitesInit struct {
// Loads the data from all of the /data folders.
data *lazy.Init

// Performs late initialization (before render) of the templates.
layouts *lazy.Init

// Loads the Git info and CODEOWNERS for all the pages if enabled.
gitInfo *lazy.Init
}
Expand Down
10 changes: 0 additions & 10 deletions hugolib/hugo_sites_build.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,10 +250,6 @@ func (h *HugoSites) process(ctx context.Context, l logg.LevelLogger, config *Bui
l = l.WithField("step", "process")
defer loggers.TimeTrackf(l, time.Now(), nil, "")

if _, err := h.init.layouts.Do(ctx); err != nil {
return err
}

if len(events) > 0 {
// This is a rebuild triggered from file events.
return h.processPartialFileEvents(ctx, l, config, init, events)
Expand Down Expand Up @@ -1067,8 +1063,6 @@ func (h *HugoSites) processPartialFileEvents(ctx context.Context, l logg.LevelLo
}

if tmplChanged || i18nChanged {
// TODO(bep) we should split this, but currently the loading of i18n and layout files are tied together. See #12048.
h.init.layouts.Reset()
if err := loggers.TimeTrackfn(func() (logg.LevelLogger, error) {
// TODO(bep) this could probably be optimized to somehow
// only load the changed templates and its dependencies, but that is non-trivial.
Expand Down Expand Up @@ -1141,10 +1135,6 @@ func (s *Site) handleContentAdapterChanges(bi pagesfromdata.BuildInfo, buildConf
}

func (h *HugoSites) processContentAdaptersOnRebuild(ctx context.Context, buildConfig *BuildCfg) error {
// Make sure the layouts are initialized.
if _, err := h.init.layouts.Do(context.Background()); err != nil {
return err
}
g := rungroup.Run[*pagesfromdata.PagesFromTemplate](ctx, rungroup.Config[*pagesfromdata.PagesFromTemplate]{
NumWorkers: h.numWorkers,
Handle: func(ctx context.Context, p *pagesfromdata.PagesFromTemplate) error {
Expand Down
5 changes: 0 additions & 5 deletions hugolib/integrationtest_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,11 +246,6 @@ func (s *IntegrationTestBuilder) AssertBuildCountGitInfo(count int) {
s.Assert(s.H.init.gitInfo.InitCount(), qt.Equals, count)
}

func (s *IntegrationTestBuilder) AssertBuildCountLayouts(count int) {
s.Helper()
s.Assert(s.H.init.layouts.InitCount(), qt.Equals, count)
}

func (s *IntegrationTestBuilder) AssertFileCount(dirname string, expected int) {
s.Helper()
fs := s.fs.WorkingDirReadOnly
Expand Down
13 changes: 9 additions & 4 deletions hugolib/page__new.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@ import (
var pageIDCounter atomic.Uint64

func (h *HugoSites) newPage(m *pageMeta) (*pageState, *paths.Path, error) {
p, pth, err := h.doNewPage(m)
if err != nil {
// Make sure that any partially created page part is marked as stale.
m.MarkStale()
}
return p, pth, err
}

func (h *HugoSites) doNewPage(m *pageMeta) (*pageState, *paths.Path, error) {
m.Staler = &resources.AtomicStaler{}
if m.pageMetaParams == nil {
m.pageMetaParams = &pageMetaParams{
Expand Down Expand Up @@ -231,10 +240,6 @@ func (h *HugoSites) newPage(m *pageMeta) (*pageState, *paths.Path, error) {
}
return ps, nil
}()
// Make sure to evict any cached and now stale data.
if err != nil {
m.MarkStale()
}

if ps == nil {
return nil, nil, err
Expand Down
10 changes: 0 additions & 10 deletions hugolib/site.go
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,6 @@ func newHugoSites(cfg deps.DepsCfg, d *deps.Deps, pageTrees *pageTrees, sites []
skipRebuildForFilenames: make(map[string]bool),
init: &hugoSitesInit{
data: lazy.New(),
layouts: lazy.New(),
gitInfo: lazy.New(),
},
}
Expand Down Expand Up @@ -400,15 +399,6 @@ func newHugoSites(cfg deps.DepsCfg, d *deps.Deps, pageTrees *pageTrees, sites []
return nil, nil
})

h.init.layouts.Add(func(context.Context) (any, error) {
for _, s := range h.Sites {
if err := s.Tmpl().(tpl.TemplateManager).MarkReady(); err != nil {
return nil, err
}
}
return nil, nil
})

h.init.gitInfo.Add(func(context.Context) (any, error) {
err := h.loadGitInfo()
if err != nil {
Expand Down
42 changes: 42 additions & 0 deletions testscripts/commands/server__error_recovery_edit_config.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# Test the hugo server command when adding an error to a config file
# and then fixing it.

hugo server &

waitServer

httpget ${HUGOTEST_BASEURL_0}p1/ 'Title: P1'

replace $WORK/hugo.toml 'title =' 'titlefoo'
httpget ${HUGOTEST_BASEURL_0}p1/ 'failed'

replace $WORK/hugo.toml 'titlefoo' 'title ='
httpget ${HUGOTEST_BASEURL_0}p1/ 'Title: P1'

stopServer

-- hugo.toml --
title = "Hugo Server Test"
baseURL = "https://example.org/"
disableKinds = ["taxonomy", "term", "sitemap"]
-- layouts/index.html --
Title: {{ .Title }}|BaseURL: {{ site.BaseURL }}|
-- layouts/_default/single.html --
Title: {{ .Title }}|BaseURL: {{ site.BaseURL }}|
-- content/_index.md --
---
title: Hugo Home
---
-- content/p1/index.md --
---
title: P1
---
-- content/p2/index.md --
---
title: P2
---
-- static/staticfiles/static.txt --
static



42 changes: 42 additions & 0 deletions testscripts/commands/server__error_recovery_edit_content.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# Test the hugo server command when adding a front matter error to a content file
# and then fixing it.

hugo server &

waitServer

httpget ${HUGOTEST_BASEURL_0}p1/ 'Title: P1'

replace $WORK/content/p1/index.md 'title:' 'titlecolon'
httpget ${HUGOTEST_BASEURL_0}p1/ 'failed'

replace $WORK/content/p1/index.md 'titlecolon' 'title:'
httpget ${HUGOTEST_BASEURL_0}p1/ 'Title: P1'

stopServer

-- hugo.toml --
title = "Hugo Server Test"
baseURL = "https://example.org/"
disableKinds = ["taxonomy", "term", "sitemap"]
-- layouts/index.html --
Title: {{ .Title }}|BaseURL: {{ site.BaseURL }}|
-- layouts/_default/single.html --
Title: {{ .Title }}|BaseURL: {{ site.BaseURL }}|
-- content/_index.md --
---
title: Hugo Home
---
-- content/p1/index.md --
---
title: P1
---
-- content/p2/index.md --
---
title: P2
---
-- static/staticfiles/static.txt --
static



1 change: 0 additions & 1 deletion tpl/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ type TemplateManager interface {
TemplateHandler
TemplateFuncGetter
AddTemplate(name, tpl string) error
MarkReady() error
}

// TemplateVariants describes the possible variants of a template.
Expand Down
Loading

0 comments on commit cb6e27b

Please sign in to comment.