Skip to content

Commit 686604b

Browse files
zulkhairTest User
andauthored
fix: mobkit build error catch (#244)
Fix mobkit / buildkit error catching <!-- greptile_comment --> ## Greptile Summary Enhances Docker build error handling and improves build process efficiency in the World CLI, particularly for BuildKit outputs and Cardinal Dockerfile configuration. - Improved BuildKit error parsing in `common/docker/client_image.go` with separate handlers for different message types (buildkit trace, v1, generic) - Optimized `common/docker/service/cardinal.Dockerfile` layer caching by separating go.mod/sum operations - Switched from mounted secret file to GITHUB_TOKEN build arg for more reliable token handling - Added detailed build progress reporting and enhanced error message detection for various output formats <!-- /greptile_comment --> Co-authored-by: Test User <test@example.com>
1 parent a596197 commit 686604b

2 files changed

Lines changed: 166 additions & 24 deletions

File tree

common/docker/client_image.go

Lines changed: 153 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -228,14 +228,19 @@ func (c *Client) addFileToTarWriter(baseDir string, tw *tar.Writer) error {
228228
}
229229

230230
// readBuildLog filters the output of the Docker build command
231-
// there is two types of output:
231+
// there are two types of output:
232232
// 1. Output from the build process without buildkit
233-
// - stream: Output from the build process
233+
// - stream: Output from the build process (steps, status, etc.)
234234
// - error: Error message from the build process
235+
// - errorDetail: Detailed error information
236+
// - status: Status updates from the build process
235237
//
236238
// 2. Output from the build process with buildkit
237-
// - moby.buildkit.trace: Output from the build process
238-
// - error: Need to research how buildkit log send error messages (TODO)
239+
// - moby.buildkit.trace: Status response with vertex information
240+
// - moby.buildkit.v1: Version 1 buildkit messages
241+
// - error: Error messages in various formats
242+
// - stream: Stream output from build steps
243+
// - progress: Progress information
239244
func (c *Client) readBuildLog(ctx context.Context, reader io.Reader, p *tea.Program, imageName string) error {
240245
// Create a new JSON decoder
241246
decoder := json.NewDecoder(reader)
@@ -295,53 +300,181 @@ func (c *Client) parseBuildkitResp(decoder *json.Decoder, stop *bool) (string, e
295300
var msg jsonmessage.JSONMessage
296301
if err := decoder.Decode(&msg); errors.Is(err, io.EOF) {
297302
*stop = true
303+
return "", nil
298304
} else if err != nil {
299305
return "", eris.Wrap(err, "Error decoding build output")
300306
}
301307

308+
// Handle different message types
309+
switch msg.ID {
310+
case "moby.buildkit.trace":
311+
return c.parseBuildkitTrace(msg)
312+
case "moby.buildkit.v1":
313+
return c.parseBuildkitV1(msg)
314+
default:
315+
// Handle other message types including errors
316+
return c.parseBuildkitGeneric(msg)
317+
}
318+
}
319+
320+
func (c *Client) parseBuildkitTrace(msg jsonmessage.JSONMessage) (string, error) {
302321
var resp controlapi.StatusResponse
303322

304-
if msg.ID != "moby.buildkit.trace" {
323+
if msg.Aux == nil {
305324
return "", nil
306325
}
307326

308327
var dt []byte
309-
// ignoring all messages that are not understood
310-
// need to research how buildkit log send error messages
311328
if err := json.Unmarshal(*msg.Aux, &dt); err != nil {
312-
return "", nil //nolint:nilerr // ignore error
329+
return "", nil //nolint:nilerr // ignore unmarshal errors for unknown message types
313330
}
314331
if err := (&resp).Unmarshal(dt); err != nil {
315-
return "", nil //nolint:nilerr // ignore error
332+
return "", nil //nolint:nilerr // ignore unmarshal errors for unknown message types
316333
}
317334

318335
if len(resp.Vertexes) == 0 {
319336
return "", nil
320337
}
321338

322-
// return the name of the vertex (step) that is currently being executed
323-
return resp.Vertexes[len(resp.Vertexes)-1].Name, nil
339+
// Return the name of the vertex (step) that is currently being executed
340+
latestVertex := resp.Vertexes[len(resp.Vertexes)-1]
341+
342+
// Check if the vertex has an error
343+
if latestVertex.Error != "" {
344+
return "", eris.New(latestVertex.Error)
345+
}
346+
347+
// Include progress information if available
348+
stepInfo := latestVertex.Name
349+
if latestVertex.ProgressGroup != nil {
350+
stepInfo = fmt.Sprintf("%s (in progress)", latestVertex.Name)
351+
}
352+
353+
return stepInfo, nil
324354
}
325355

326-
func (c *Client) parseNonBuildkitResp(decoder *json.Decoder, stop *bool) (string, error) {
356+
func (c *Client) parseBuildkitV1(msg jsonmessage.JSONMessage) (string, error) {
357+
// Handle buildkit v1 messages which can contain detailed build information
358+
if msg.Aux == nil {
359+
return "", nil
360+
}
361+
362+
var auxData map[string]interface{}
363+
if err := json.Unmarshal(*msg.Aux, &auxData); err != nil {
364+
return "", nil //nolint:nilerr // ignore unmarshal errors
365+
}
366+
367+
// Extract step information from v1 messages
368+
if step, ok := auxData["step"].(string); ok && step != "" {
369+
return step, nil
370+
}
371+
372+
// Check for error information
373+
if errorMsg, ok := auxData["error"].(string); ok && errorMsg != "" {
374+
return "", eris.New(errorMsg)
375+
}
376+
377+
return "", nil
378+
}
379+
380+
func (c *Client) parseBuildkitGeneric(msg jsonmessage.JSONMessage) (string, error) {
381+
// Handle generic buildkit messages including errors and progress updates
382+
383+
// Check for error messages in the main message
384+
if msg.Error != nil {
385+
return "", eris.New(msg.Error.Message)
386+
}
387+
388+
// Check for error messages in the stream
389+
if msg.Stream != "" {
390+
stream := strings.TrimSpace(msg.Stream)
391+
392+
// Check if this is an error message
393+
if strings.Contains(strings.ToLower(stream), "error") {
394+
return "", eris.New(stream)
395+
}
396+
397+
// Check if this is a build step
398+
if strings.HasPrefix(stream, "Step") {
399+
return stream, nil
400+
}
401+
402+
// Check for other important build information
403+
if strings.Contains(stream, "Pulling") ||
404+
strings.Contains(stream, "Building") ||
405+
strings.Contains(stream, "Running") ||
406+
strings.Contains(stream, "Executing") {
407+
return stream, nil
408+
}
409+
}
410+
411+
// Check for error details
412+
if msg.Error != nil {
413+
return "", eris.New(msg.Error.Message)
414+
}
415+
416+
// Check for progress information
417+
if msg.Progress != nil {
418+
progress := msg.Progress.String()
419+
if progress != "" {
420+
return progress, nil
421+
}
422+
}
423+
424+
return "", nil
425+
}
426+
427+
func (c *Client) parseNonBuildkitResp(decoder *json.Decoder, stop *bool) (string, error) { //nolint:gocognit
327428
var event map[string]interface{}
328429
if err := decoder.Decode(&event); errors.Is(err, io.EOF) {
329430
*stop = true
431+
return "", nil
330432
} else if err != nil {
331433
return "", eris.Wrap(err, "Error decoding build output")
332434
}
333435

334-
// Only show the step if it's a build step
335-
step := ""
336-
if val, ok := event["stream"]; ok && val != "" && strings.HasPrefix(val.(string), "Step") {
337-
if step, ok = val.(string); ok && step != "" {
338-
step = strings.TrimSpace(step)
339-
}
340-
} else if val, ok = event["error"]; ok && val != "" {
436+
// Check for error messages first
437+
if val, ok := event["error"]; ok && val != "" {
341438
return "", eris.New(val.(string))
342439
}
343440

344-
return step, nil
441+
// Check for error details
442+
if errorDetail, ok := event["errorDetail"]; ok {
443+
if errorDetailMap, ok := errorDetail.(map[string]interface{}); ok { //nolint:govet
444+
if message, ok := errorDetailMap["message"]; ok && message != "" { //nolint:govet
445+
return "", eris.New(message.(string))
446+
}
447+
}
448+
}
449+
450+
// Check for build steps and other important information
451+
if val, ok := event["stream"]; ok && val != "" {
452+
stream := strings.TrimSpace(val.(string))
453+
454+
// Check if this is a build step
455+
if strings.HasPrefix(stream, "Step") {
456+
return stream, nil
457+
}
458+
459+
// Check for other important build information
460+
if strings.Contains(stream, "Pulling") ||
461+
strings.Contains(stream, "Building") ||
462+
strings.Contains(stream, "Running") ||
463+
strings.Contains(stream, "Executing") ||
464+
strings.Contains(stream, "Successfully") {
465+
return stream, nil
466+
}
467+
}
468+
469+
// Check for status updates
470+
if val, ok := event["status"]; ok && val != "" {
471+
status := strings.TrimSpace(val.(string))
472+
if status != "" {
473+
return status, nil
474+
}
475+
}
476+
477+
return "", nil
345478
}
346479

347480
// filterImages filters the images that need to be pulled.

common/docker/service/cardinal.Dockerfile

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,26 @@ ENV GOPRIVATE=github.com/argus-labs/*,pkg.world.dev/*
1313

1414
# Configure git to use HTTPS with GitHub token
1515
RUN --mount=type=secret,id=github_token \
16-
git config --global url."https://$(cat /run/secrets/github_token):x-oauth-basic@github.com/".insteadOf "https://github.com/"
16+
git config --global url."https://${GITHUB_TOKEN}:x-oauth-basic@github.com/".insteadOf "https://github.com/"
1717

18-
# Copy the entire source code
19-
COPY /${SOURCE_PATH} ./
18+
# Copy go.mod files first to leverage Docker layer caching
19+
COPY /${SOURCE_PATH}/go.mod ./
2020

2121
# Download dependencies
22-
RUN go mod tidy
22+
RUN go mod download
2323

2424
# Set the GOCACHE environment variable to /root/.cache/go-build to speed up build
2525
ENV GOCACHE=/root/.cache/go-build
2626

27+
# Copy the entire source code
28+
COPY /${SOURCE_PATH} ./
29+
30+
# Remove go.sum file
31+
RUN rm go.sum
32+
33+
# Run go mod tidy to remove unused dependencies
34+
RUN go mod tidy
35+
2736
# Build the binary
2837
RUN --mount=type=cache,target="/root/.cache/go-build" go build -v -o /go/bin/app
2938

0 commit comments

Comments
 (0)