Skip to content

Commit 9888095

Browse files
authored
Surface error even if we cannot parse ERROR lines in the output of restore (#771)
Fix `RunPGRestore` producing `error restoring dump: %!w(<nil>)` when the command fails with a non-zero exit code but the output contains no parseable ERROR lines. Now falls back to the exec error instead of wrapping nil.
1 parent cb0d45d commit 9888095

2 files changed

Lines changed: 76 additions & 2 deletions

File tree

internal/postgres/pg_restore.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,13 +92,26 @@ func RunPGRestore(ctx context.Context, opts PGRestoreOptions, dump []byte) (stri
9292

9393
// TODO: add streaming support when large data output is required
9494
out, err := cmd.CombinedOutput()
95-
if err != nil || strings.Contains(string(out), "ERROR") {
96-
return "", fmt.Errorf("error restoring dump: %w", parsePgRestoreOutputErrs(out))
95+
if restoreErr := buildRestoreError(out, err); restoreErr != nil {
96+
return "", restoreErr
9797
}
9898

9999
return string(out), nil
100100
}
101101

102+
func buildRestoreError(out []byte, execErr error) error {
103+
if execErr == nil && !strings.Contains(string(out), "ERROR") {
104+
return nil
105+
}
106+
if parseErr := parsePgRestoreOutputErrs(out); parseErr != nil {
107+
return fmt.Errorf("error restoring dump: %w", parseErr)
108+
}
109+
if execErr != nil {
110+
return fmt.Errorf("error restoring dump: %w", execErr)
111+
}
112+
return nil
113+
}
114+
102115
func removeDatabaseFromConnectionString(url string) (string, error) {
103116
dbName, err := extractDatabase(url)
104117
if err != nil {

internal/postgres/pg_restore_test.go

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,67 @@ pg_restore: finished`,
149149
}
150150
}
151151

152+
func TestBuildRestoreError(t *testing.T) {
153+
t.Parallel()
154+
155+
execErr := errors.New("exit status 1")
156+
157+
tests := []struct {
158+
name string
159+
output []byte
160+
execErr error
161+
162+
wantNil bool
163+
wantContain string
164+
}{
165+
{
166+
name: "no error - success",
167+
output: []byte("pg_restore: finished\n"),
168+
execErr: nil,
169+
wantNil: true,
170+
},
171+
{
172+
name: "exec error with no parseable output",
173+
output: []byte("some unexpected output\n"),
174+
execErr: execErr,
175+
wantContain: "exit status 1",
176+
},
177+
{
178+
name: "exec error with empty output",
179+
output: []byte{},
180+
execErr: execErr,
181+
wantContain: "exit status 1",
182+
},
183+
{
184+
name: "exec error with parseable ERROR lines",
185+
output: []byte("pg_restore: error: could not execute query: ERROR: relation \"users\" already exists\n"),
186+
execErr: execErr,
187+
wantContain: "already exists",
188+
},
189+
{
190+
name: "no exec error but output contains ERROR",
191+
output: []byte("ERROR: relation \"users\" already exists\n"),
192+
execErr: nil,
193+
wantContain: "already exists",
194+
},
195+
}
196+
197+
for _, tc := range tests {
198+
t.Run(tc.name, func(t *testing.T) {
199+
t.Parallel()
200+
201+
err := buildRestoreError(tc.output, tc.execErr)
202+
if tc.wantNil {
203+
require.NoError(t, err)
204+
return
205+
}
206+
require.Error(t, err)
207+
assert.Contains(t, err.Error(), tc.wantContain)
208+
assert.NotContains(t, err.Error(), "%!w(<nil>)")
209+
})
210+
}
211+
}
212+
152213
func TestIsErrorLine(t *testing.T) {
153214
tests := []struct {
154215
line string

0 commit comments

Comments
 (0)