Skip to content

modules: Improve error checking #14014

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

modules: Improve error checking #14014

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
modules: Improve error checking 
1. Validate combination of module path and version
2. Capture and display error messages from JSON sent to stdout
3. Remove invalid semver from mod_vendor__versions.txt testscript

Closes #14010
  • Loading branch information
@jmooring
jmooring committed Sep 29, 2025
commit 4460b0f420a249ff3c01498dabcab8b48d051958
21 changes: 15 additions & 6 deletions modules/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -717,12 +717,13 @@ func (c *Client) runGo(
return nil
}

stderr := new(bytes.Buffer)
stdErrBuf := new(bytes.Buffer)
stdOutBuf := new(bytes.Buffer)

argsv := collections.StringSliceToInterfaceSlice(args)
argsv = append(argsv, hexec.WithEnviron(c.environ))
argsv = append(argsv, hexec.WithStderr(goOutputReplacerWriter{w: io.MultiWriter(stderr, os.Stderr)}))
argsv = append(argsv, hexec.WithStdout(stdout))
argsv = append(argsv, hexec.WithStderr(goOutputReplacerWriter{w: io.MultiWriter(stdErrBuf, os.Stderr)}))
argsv = append(argsv, hexec.WithStdout(goOutputReplacerWriter{w: io.MultiWriter(stdOutBuf, stdout)}))
argsv = append(argsv, hexec.WithDir(c.ccfg.WorkingDir))
argsv = append(argsv, hexec.WithContext(ctx))

Expand All @@ -737,7 +738,15 @@ func (c *Client) runGo(
return nil
}

if strings.Contains(stderr.String(), "invalid version: unknown revision") {
var stdOutMap map[string]any
err2 := json.Unmarshal(stdOutBuf.Bytes(), &stdOutMap)
if err2 == nil {
if stdOutMap["Error"] != "" {
return fmt.Errorf("%s", stdOutMap["Error"])
}
}

if strings.Contains(stdErrBuf.String(), "invalid version: unknown revision") {
// See https://github.com/gohugoio/hugo/issues/6825
c.logger.Println(`An unknown revision most likely means that someone has deleted the remote ref (e.g. with a force push to GitHub).
To resolve this, you need to manually edit your go.mod file and replace the version for the module in question with a valid ref.
Expand All @@ -755,12 +764,12 @@ If you then run 'hugo mod graph' it should resolve itself to the most recent ver
}

// Too old Go version
if strings.Contains(stderr.String(), "flag provided but not defined") {
if strings.Contains(stdErrBuf.String(), "flag provided but not defined") {
c.goBinaryStatus = goBinaryStatusTooOld
return nil
}

return fmt.Errorf("go command failed: %s", stderr)
return fmt.Errorf("go command failed: %s", stdErrBuf.String())

}

Expand Down
8 changes: 8 additions & 0 deletions modules/collect.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,14 @@ func (c *collector) addAndRecurse(owner *moduleAdapter) error {
continue
}

// Validate the combination of module path and version.
if moduleImport.Version != "" {
err := module.Check(moduleImport.Path, moduleImport.Version)
Copy link
Member

Choose a reason for hiding this comment

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

I understand this i a draft, and I'm not totally sure what the above check does, but for version when path points to a non-Go-module, a path that does not end with /v2 may point at a v2.0.0 version. I tested this earlier today with:

https://github.com/bep/hugomodnogomod
Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, still looking into this. The intent is to prevent invalid semvers and invalid path/semver pairs such as:

path = "github.com/user/project/v3"
version = 'v42.6.7'

path points to a non-Go-module

Thanks. I will probably use https://github.com/bep/hugomodnogomod in an integration test unless this is a temporary repo.
Copy link
Member

Choose a reason for hiding this comment

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

unless this is a temporary repo.

I will let it live, I will probably have a use for it myself some day.
Copy link
Member Author

@jmooring jmooring Sep 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Note to self: there isn't a simple and reliable way to verify that the combination of a module path and version query is semantically correct before attempting to get, list, etc.
if err != nil {
return err
}
}

// Prevent cyclic references.
if v := c.isPathSeen(moduleImport.Path, owner); v != nil && v != owner {
continue
Expand Down
48 changes: 48 additions & 0 deletions modules/modules_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@
package modules_test

import (
"strings"
"testing"

qt "github.com/frankban/quicktest"
"github.com/gohugoio/hugo/hugolib"
)

Expand Down Expand Up @@ -48,3 +50,49 @@ Deps: {{ range hugo.Deps}}{{ printf "%s@%s" .Path .Version }}|{{ end }}$
b.AssertFileContent("public/blog/music/autumn-leaves/index.html", "Autumn Leaves is a popular jazz standard") // v0.2.0
b.AssertFileContent("public/v1/blog/music/autumn-leaves/index.html", "Lorem markdownum, placidi peremptis") // v0.1.0
}

// Issue 14010
func TestModuleImportErrors(t *testing.T) {
t.Parallel()

files := `
-- hugo.toml --
[[module.imports]]
PATH
VERSION
`
f := strings.NewReplacer("PATH", "", "VERSION", "").Replace(files)
b, err := hugolib.TestE(t, f)
b.Assert(err, qt.IsNotNil)
b.Assert(err, qt.ErrorMatches, `^failed to load modules: module "" not found.*`)

f = strings.NewReplacer("PATH", "path = 'foo'", "VERSION", "").Replace(files)
b, err = hugolib.TestE(t, f)
b.Assert(err, qt.IsNotNil)
b.Assert(err, qt.ErrorMatches, `^failed to load modules: module "foo" not found.*`)

f = strings.NewReplacer("PATH", "path = 'foo'", "VERSION", "version = 'badSemVer'").Replace(files)
b, err = hugolib.TestE(t, f)
b.Assert(err, qt.IsNotNil)
b.Assert(err, qt.ErrorMatches, `failed to load modules: malformed module path "foo": missing dot in first path element`)

f = strings.NewReplacer("PATH", "path = 'foo.bar'", "VERSION", "version = 'badSemVer'").Replace(files)
b, err = hugolib.TestE(t, f)
b.Assert(err, qt.IsNotNil)
b.Assert(err, qt.ErrorMatches, `failed to load modules: foo.bar@badSemVer: invalid version: not a semantic version`)

f = strings.NewReplacer("PATH", "path = 'foo.bar'", "VERSION", "version = 'v6.7.42'").Replace(files)
b, err = hugolib.TestE(t, f)
b.Assert(err, qt.IsNotNil)
b.Assert(err, qt.ErrorMatches, `failed to load modules: foo.bar@v6.7.42: invalid version: should be v0 or v1, not v6`)

f = strings.NewReplacer("PATH", "path = 'foo.bar/v2'", "VERSION", "version = 'v6.7.42'").Replace(files)
b, err = hugolib.TestE(t, f)
b.Assert(err, qt.IsNotNil)
b.Assert(err, qt.ErrorMatches, `failed to load modules: foo.bar/v2@v6.7.42: invalid version: should be v2, not v6`)

f = strings.NewReplacer("PATH", "path = 'github.com/bep/hugo-mod-misc/dummy-content/v99'", "VERSION", "version = 'v99.0.0'").Replace(files)
b, err = hugolib.TestE(t, f)
b.Assert(err, qt.IsNotNil)
b.Assert(err, qt.ErrorMatches, `failed to load modules: failed to download module github.com/bep/hugo-mod-misc/dummy-content/v99@v99.0.0: github.com/bep/hugo-mod-misc/dummy-content/v99@v99.0.0: invalid version: unknown revision dummy-content/v99.0.0`)
}
14 changes: 7 additions & 7 deletions testscripts/commands/mod_vendor__versions.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ dostounix golden/vendor.txt
hugo mod vendor
cmp _vendor/modules.txt golden/vendor.txt
lsr _vendor
stdout 'github.com/bep/hugo-mod-misc/dummy-content@%3C%3Dv0.1.0/config.toml'
stdout 'github.com/bep/hugo-mod-misc/dummy-content@v0.1.0/config.toml'
stdout 'github.com/bep/hugo-mod-misc/dummy-content@v0.2.0/config.toml'
stdout 'github.com/bep/hugo-mod-misc/dummy-content@v0.2.0/content/blog/music/all-of-me/index.md'
stdout 'github.com/bep/hugo-mod-misc/dummy-content@v0.2.0/content/blog/music/blue-bossa/index.md'
stdout 'github.com/bep/hugo-mod-misc/dummy-content@%3C%3Dv0.1.0/content/blog/music/all-of-me/index.md'
! stdout 'github.com/bep/hugo-mod-misc/dummy-content@%3C%3Dv0.1.0/content/blog/music/blue-bossa/index.md' # not mounted
stdout 'github.com/bep/hugo-mod-misc/dummy-content@v0.1.0/content/blog/music/all-of-me/index.md'
! stdout 'github.com/bep/hugo-mod-misc/dummy-content@v0.1.0/content/blog/music/blue-bossa/index.md' # not mounted

hugo mod graph
stdout 'project github.com/bep/hugo-mod-misc/dummy-content@v0.2.0'
stdout 'project github.com/bep/hugo-mod-misc/dummy-content@v0.2.0'
stdout 'project github.com/bep/hugo-mod-misc/dummy-content@v0.1.0'

hugo config mounts
Expand All @@ -24,7 +24,7 @@ path = "github.com/bep/hugo-mod-misc/dummy-content"
version = "v0.2.0"
[[module.imports]]
path = "github.com/bep/hugo-mod-misc/dummy-content"
version = "<=v0.1.0"
version = "v0.1.0"
[[module.imports.mounts]]
source = "content/blog/music/all-of-me"
target = "content/all"
Expand All @@ -35,7 +35,7 @@ Deps: {{ range hugo.Deps}}{{ printf "%s@%s" .Path .Version }}|{{ end }}$

-- golden/vendor.txt --
# github.com/bep/hugo-mod-misc/dummy-content@v0.2.0 v0.2.0
# github.com/bep/hugo-mod-misc/dummy-content@%3C%3Dv0.1.0 v0.1.0
# github.com/bep/hugo-mod-misc/dummy-content@v0.1.0 v0.1.0
-- golden/mounts.json --
{
"path": "project",
Expand Down Expand Up @@ -92,7 +92,7 @@ Deps: {{ range hugo.Deps}}{{ printf "%s@%s" .Path .Version }}|{{ end }}$
"version": "v0.1.0",
"time": "0001-01-01T00:00:00Z",
"owner": "project",
"dir": "$WORK/_vendor/github.com/bep/hugo-mod-misc/dummy-content@%3C%3Dv0.1.0/",
"dir": "$WORK/_vendor/github.com/bep/hugo-mod-misc/dummy-content@v0.1.0/",
"mounts": [
{
"source": "content/blog/music/all-of-me",
Expand Down
Loading