-
Notifications
You must be signed in to change notification settings - Fork 0
feat: surface initError on the build when deploy fails #71
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -485,7 +485,7 @@ | |
| if ui.IsTerminalStatus(msg.status) { | ||
| // Terminal status detected, trigger completion | ||
| return m, func() tea.Msg { | ||
| return buildCompleteMsg{status: msg.status} | ||
| return buildCompleteMsg{status: msg.status, initError: msg.initError} | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -506,7 +506,7 @@ | |
|
|
||
| // Wait 2 seconds to allow remaining logs to arrive | ||
| return m, tea.Tick(2*time.Second, func(t time.Time) tea.Msg { | ||
| return logDrainCompleteMsg(msg) | ||
| return logDrainCompleteMsg{status: msg.status, initError: msg.initError} | ||
| }) | ||
|
|
||
| case logDrainCompleteMsg: | ||
|
|
@@ -563,15 +563,23 @@ | |
|
|
||
| if m.conf.SimpleOutput() { | ||
| fmt.Printf("✗ Build failed with status: %s\n", msg.status) | ||
| if msg.initError != nil && *msg.initError != "" { | ||
| fmt.Println() | ||
| fmt.Println(*msg.initError) | ||
| } | ||
| return m, tea.Quit | ||
| } | ||
|
|
||
| // Print error message to scrollback in interactive mode | ||
| return m, tea.Sequence( | ||
| cmds := []tea.Cmd{ | ||
| tea.Println(""), | ||
| tea.Println(ui.ErrorStyle.Render(fmt.Sprintf("✗ Build failed with status: %s", msg.status))), | ||
| tea.Quit, | ||
| ) | ||
| } | ||
| if msg.initError != nil && *msg.initError != "" { | ||
| cmds = append(cmds, tea.Println(""), tea.Println(*msg.initError)) | ||
| } | ||
| cmds = append(cmds, tea.Quit) | ||
| return m, tea.Sequence(cmds...) | ||
| } | ||
|
|
||
| case confirmationResponseMsg: | ||
|
|
@@ -880,20 +888,23 @@ | |
| type zipUploadedMsg struct{} | ||
|
|
||
| type buildStatusUpdateMsg struct { | ||
| buildID string | ||
| status string | ||
| buildID string | ||
| status string | ||
| initError *string | ||
| } | ||
|
|
||
| type buildStatusPollErrorMsg struct { | ||
| err error | ||
| } | ||
|
|
||
| type buildCompleteMsg struct { | ||
| status string | ||
| status string | ||
| initError *string | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather just use if msg.initError != nil && *msg.initError != ""let's just simplify and do if msg.initError != ""
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So the reason it's tempting (I think) to put a pointer on the type is to be able to distinguish between 'present but empty' and 'not present'. I think that distinction is only useful if there's a functional difference between those two things. In this case as far as I can tell there is no functional difference so let's just keep it to non-pointer. It makes the type a lot easier to work with |
||
| } | ||
|
|
||
| type logDrainCompleteMsg struct { | ||
| status string // Final build status to use for completion | ||
| status string // Final build status to use for completion | ||
| initError *string // User-facing error message persisted on the build | ||
| } | ||
|
|
||
| type buildCancelledMsg struct { | ||
|
|
@@ -1213,8 +1224,9 @@ | |
|
|
||
| // Return status update message | ||
| return buildStatusUpdateMsg{ | ||
| buildID: build.Id, | ||
| status: build.Status, | ||
| buildID: build.Id, | ||
| status: build.Status, | ||
| initError: build.InitError, | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As below