From daef1246b8fcffa3317cc612d226ab580b4a58fb Mon Sep 17 00:00:00 2001 From: Rick Guo Date: Fri, 24 Jul 2026 15:05:28 +0800 Subject: [PATCH 1/3] refactor(install): parse llard responses with cmdjsonl --- cmd/llar/internal/install.go | 80 ++++++++++--------------------- cmd/llar/internal/install_test.go | 12 ++--- 2 files changed, 32 insertions(+), 60 deletions(-) diff --git a/cmd/llar/internal/install.go b/cmd/llar/internal/install.go index a9d1a17..9c294a2 100644 --- a/cmd/llar/internal/install.go +++ b/cmd/llar/internal/install.go @@ -3,8 +3,6 @@ package internal import ( "bufio" "context" - "encoding/json" - "errors" "fmt" "io" "mime" @@ -20,6 +18,7 @@ import ( buildcache "github.com/goplus/llar/internal/build/cache" "github.com/goplus/llar/internal/metadata" "github.com/goplus/llar/mod/module" + "github.com/qiniu/x/cmdjsonl" "github.com/spf13/cobra" ) @@ -222,60 +221,33 @@ func requestInstallArtifacts(ctx context.Context, progress io.Writer, client *ht } var artifacts []installArtifactMessage - // TODO: Upgrade ixgo and replace this parser with github.com/qiniu/x/cmdjsonl. - // - // Dependency constraint: - // - ixgo v0.61.0 ships generated bindings for qiniu/x packages such as - // gsh, osx, stringutil, and xgo/ng. - // - Its stringutil binding references Builder, NewBuilder, and - // NewBuilderSize. - // - cmdjsonl first appears in qiniu/x v1.17.1, which removed those - // stringutil APIs, so upgrading qiniu/x alone breaks the build. - // - // Migration: - // - Upgrade ixgo to bindings compatible with qiniu/x v1.17.1 or newer. - // - Replace this temporary parser with cmdjsonl.Parser. - reader := bufio.NewReader(resp.Body) - for lineNo := 1; ; lineNo++ { - line, readErr := reader.ReadString('\n') - line = strings.TrimSuffix(strings.TrimSuffix(line, "\n"), "\r") - if line != "" { - command, data, ok := strings.Cut(line, " ") - if !ok { - return nil, fmt.Errorf("invalid llard response line %d", lineNo) - } - switch command { - case "info": - var message string - if err := json.Unmarshal([]byte(data), &message); err != nil { - return nil, fmt.Errorf("decode llard info line %d: %w", lineNo, err) - } - fmt.Fprintln(progress, message) - case "error": - var message string - if err := json.Unmarshal([]byte(data), &message); err != nil { - return nil, fmt.Errorf("decode llard error line %d: %w", lineNo, err) - } - return nil, fmt.Errorf("llard: %s", message) - case "artifact": - var artifact installArtifactMessage - if err := json.Unmarshal([]byte(data), &artifact); err != nil { - return nil, fmt.Errorf("decode llard artifact line %d: %w", lineNo, err) - } - if artifact.ID == "" || artifact.Type == "" || artifact.URL == "" { - return nil, fmt.Errorf("invalid llard artifact line %d", lineNo) - } - artifacts = append(artifacts, artifact) - default: - return nil, fmt.Errorf("unsupported llard response command %q", command) - } + var responseErr error + var parser cmdjsonl.Parser + if err := parser.HandleFunc("info", func(message string) { + fmt.Fprintln(progress, message) + }); err != nil { + return nil, err + } + if err := parser.HandleFunc("error", func(message string) error { + responseErr = fmt.Errorf("llard: %s", message) + return responseErr + }); err != nil { + return nil, err + } + if err := parser.HandleFunc("artifact", func(artifact installArtifactMessage) error { + if artifact.ID == "" || artifact.Type == "" || artifact.URL == "" { + return fmt.Errorf("invalid llard artifact") } - if readErr != nil { - if errors.Is(readErr, io.EOF) { - break - } - return nil, readErr + artifacts = append(artifacts, artifact) + return nil + }); err != nil { + return nil, err + } + if err := parser.Parse(resp.Body, bufio.MaxScanTokenSize); err != nil { + if responseErr != nil { + return nil, responseErr } + return nil, err } if len(artifacts) == 0 { return nil, fmt.Errorf("llard returned no artifacts") diff --git a/cmd/llar/internal/install_test.go b/cmd/llar/internal/install_test.go index 7b965e3..114c274 100644 --- a/cmd/llar/internal/install_test.go +++ b/cmd/llar/internal/install_test.go @@ -569,12 +569,12 @@ func TestRequestInstallArtifactsRejectsInvalidResponses(t *testing.T) { }{ {name: "status", status: http.StatusServiceUnavailable, contentType: "application/x-cmdjsonl", want: "llard returned 503 Service Unavailable"}, {name: "content type", contentType: "text/plain", body: "artifact {}\n", want: `llard returned content type "text/plain", want application/x-cmdjsonl`}, - {name: "line", contentType: "application/x-cmdjsonl", body: "invalid\n", want: "invalid llard response line 1"}, - {name: "info JSON", contentType: "application/x-cmdjsonl", body: "info {\n", want: "decode llard info line 1"}, - {name: "error JSON", contentType: "application/x-cmdjsonl", body: "error {\n", want: "decode llard error line 1"}, - {name: "artifact JSON", contentType: "application/x-cmdjsonl", body: "artifact {\n", want: "decode llard artifact line 1"}, - {name: "artifact fields", contentType: "application/x-cmdjsonl", body: "artifact {\"id\":\"test/root@v1?os=linux\"}\n", want: "invalid llard artifact line 1"}, - {name: "command", contentType: "application/x-cmdjsonl", body: "done {}\n", want: `unsupported llard response command "done"`}, + {name: "line", contentType: "application/x-cmdjsonl", body: "invalid\n", want: "1: parse error when ParseCommand: no space found"}, + {name: "info JSON", contentType: "application/x-cmdjsonl", body: "info {\n", want: "1: parse error when UnmarshalParam: unexpected end of JSON input"}, + {name: "error JSON", contentType: "application/x-cmdjsonl", body: "error {\n", want: "1: parse error when UnmarshalParam: unexpected end of JSON input"}, + {name: "artifact JSON", contentType: "application/x-cmdjsonl", body: "artifact {\n", want: "1: parse error when UnmarshalParam: unexpected end of JSON input"}, + {name: "artifact fields", contentType: "application/x-cmdjsonl", body: "artifact {\"id\":\"test/root@v1?os=linux\"}\n", want: "1: parse error when CallHandler artifact: invalid llard artifact"}, + {name: "command", contentType: "application/x-cmdjsonl", body: "done {}\n", want: "1: parse error when ParseCommand: unknown command 'done'"}, {name: "no artifacts", contentType: "application/x-cmdjsonl", body: "info \"checking\"\n", want: "llard returned no artifacts"}, } for _, tt := range tests { From 493e64058741a5399fbbfb12a27191813f3f4a62 Mon Sep 17 00:00:00 2001 From: Rick Guo Date: Wed, 29 Jul 2026 14:34:09 +0800 Subject: [PATCH 2/3] test(install): cover cmdjsonl transport and size errors --- cmd/llar/internal/install_test.go | 40 +++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/cmd/llar/internal/install_test.go b/cmd/llar/internal/install_test.go index 114c274..ffd96ab 100644 --- a/cmd/llar/internal/install_test.go +++ b/cmd/llar/internal/install_test.go @@ -1,6 +1,7 @@ package internal import ( + "bufio" "bytes" "context" "encoding/json" @@ -569,6 +570,7 @@ func TestRequestInstallArtifactsRejectsInvalidResponses(t *testing.T) { }{ {name: "status", status: http.StatusServiceUnavailable, contentType: "application/x-cmdjsonl", want: "llard returned 503 Service Unavailable"}, {name: "content type", contentType: "text/plain", body: "artifact {}\n", want: `llard returned content type "text/plain", want application/x-cmdjsonl`}, + {name: "content type syntax", contentType: "application/x-cmdjsonl; invalid", body: "artifact {}\n", want: `llard returned content type "application/x-cmdjsonl; invalid", want application/x-cmdjsonl`}, {name: "line", contentType: "application/x-cmdjsonl", body: "invalid\n", want: "1: parse error when ParseCommand: no space found"}, {name: "info JSON", contentType: "application/x-cmdjsonl", body: "info {\n", want: "1: parse error when UnmarshalParam: unexpected end of JSON input"}, {name: "error JSON", contentType: "application/x-cmdjsonl", body: "error {\n", want: "1: parse error when UnmarshalParam: unexpected end of JSON input"}, @@ -604,6 +606,44 @@ func TestRequestInstallArtifactsRejectsInvalidResponses(t *testing.T) { } } +func TestRequestInstallArtifactsReturnsTransportError(t *testing.T) { + baseURL, err := url.Parse("https://llard.example") + if err != nil { + t.Fatal(err) + } + client := &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + return nil, errors.New("connection reset") + })} + + _, err = requestInstallArtifacts( + context.Background(), nil, client, baseURL, + module.Version{Path: "test/root"}, url.Values{"os": {"linux"}}, + ) + if err == nil || !strings.Contains(err.Error(), "connection reset") { + t.Fatalf("requestInstallArtifacts() error = %v, want transport error", err) + } +} + +func TestRequestInstallArtifactsRejectsOversizedResponse(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/x-cmdjsonl") + _, _ = io.WriteString(w, strings.Repeat("x", bufio.MaxScanTokenSize+1)+"\n") + })) + defer server.Close() + + baseURL, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + _, err = requestInstallArtifacts( + context.Background(), nil, server.Client(), baseURL, + module.Version{Path: "test/root"}, url.Values{"os": {"linux"}}, + ) + if err == nil || err.Error() != "1: parse error when ReadLine: line too long" { + t.Fatalf("requestInstallArtifacts() error = %v, want oversized line error", err) + } +} + func TestDownloadInstallArtifactResponses(t *testing.T) { archive := makeInstallArtifact(t, ".zip", "include/root.h", "root", "/build/root", "-lroot", nil) server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { From 974ae19a856a4cb3c79c6201b6cbdfa5b429c935 Mon Sep 17 00:00:00 2001 From: Rick Guo Date: Wed, 29 Jul 2026 14:48:48 +0800 Subject: [PATCH 3/3] fix(install): preserve multiple llard errors --- cmd/llar/internal/install.go | 19 ++++++++++--------- cmd/llar/internal/install_test.go | 14 ++++++++++++++ 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/cmd/llar/internal/install.go b/cmd/llar/internal/install.go index 9c294a2..4370da0 100644 --- a/cmd/llar/internal/install.go +++ b/cmd/llar/internal/install.go @@ -3,6 +3,7 @@ package internal import ( "bufio" "context" + "errors" "fmt" "io" "mime" @@ -221,16 +222,15 @@ func requestInstallArtifacts(ctx context.Context, progress io.Writer, client *ht } var artifacts []installArtifactMessage - var responseErr error + var responseErrs []error var parser cmdjsonl.Parser if err := parser.HandleFunc("info", func(message string) { fmt.Fprintln(progress, message) }); err != nil { return nil, err } - if err := parser.HandleFunc("error", func(message string) error { - responseErr = fmt.Errorf("llard: %s", message) - return responseErr + if err := parser.HandleFunc("error", func(message string) { + responseErrs = append(responseErrs, fmt.Errorf("llard: %s", message)) }); err != nil { return nil, err } @@ -243,11 +243,12 @@ func requestInstallArtifacts(ctx context.Context, progress io.Writer, client *ht }); err != nil { return nil, err } - if err := parser.Parse(resp.Body, bufio.MaxScanTokenSize); err != nil { - if responseErr != nil { - return nil, responseErr - } - return nil, err + parseErr := parser.Parse(resp.Body, bufio.MaxScanTokenSize) + if len(responseErrs) > 0 { + return nil, errors.Join(responseErrs...) + } + if parseErr != nil { + return nil, parseErr } if len(artifacts) == 0 { return nil, fmt.Errorf("llard returned no artifacts") diff --git a/cmd/llar/internal/install_test.go b/cmd/llar/internal/install_test.go index ffd96ab..69acb84 100644 --- a/cmd/llar/internal/install_test.go +++ b/cmd/llar/internal/install_test.go @@ -560,6 +560,20 @@ func TestInstallReturnsLlardError(t *testing.T) { } } +func TestInstallReturnsMultipleLlardErrors(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/x-cmdjsonl") + writeInstallCommand(t, w, "error", "build failed") + writeInstallCommand(t, w, "error", "test failed") + })) + defer server.Close() + + _, err := install(context.Background(), nil, server.URL, "test/root@v1.0.0", hostMatrix()) + if err == nil || err.Error() != "llard: build failed\nllard: test failed" { + t.Fatalf("install() error = %v, want joined llard errors", err) + } +} + func TestRequestInstallArtifactsRejectsInvalidResponses(t *testing.T) { tests := []struct { name string