Skip to content

Commit 7b228df

Browse files
committed
testserver: improve the error trace
Wrapped errors from `NewTestServer`and `NewTenantServer`. Also improved logging for better error trace by adding "cockroach-go testserver:" or `cockroach-go tenantserver:` at the beginning of log messages.
1 parent fb73350 commit 7b228df

File tree

3 files changed

+61
-49
lines changed

3 files changed

+61
-49
lines changed

testserver/binaries.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ const (
4242
func downloadFile(response *http.Response, filePath string, tc *TestConfig) error {
4343
output, err := os.OpenFile(filePath, os.O_WRONLY|os.O_CREATE|os.O_EXCL, writingFileMode)
4444
if err != nil {
45-
return fmt.Errorf("error creating %s: %s", filePath, err)
45+
return fmt.Errorf("error creating %s: %w", filePath, err)
4646
}
4747
defer func() { _ = output.Close() }()
4848

@@ -66,7 +66,7 @@ func downloadFile(response *http.Response, filePath string, tc *TestConfig) erro
6666
}
6767

6868
if _, err := io.Copy(output, response.Body); err != nil {
69-
return fmt.Errorf("problem saving %s to %s: %s", response.Request.URL, filePath, err)
69+
return fmt.Errorf("problem saving %s to %s: %w", response.Request.URL, filePath, err)
7070
}
7171

7272
// Download was successful, add the rw bits.
@@ -97,7 +97,7 @@ func GetDownloadResponse() (*http.Response, error) {
9797
cmd := exec.Command("ldd", "--version")
9898
out, err := cmd.Output()
9999
if err != nil {
100-
log.Printf("%s: out=%q err=%s", cmd.Args, out, err)
100+
log.Printf("%s: %s: out=%q err=%v", testserverMessagePrefix, cmd.Args, out, err)
101101
} else if muslRE.Match(out) {
102102
return "-musl"
103103
}
@@ -192,7 +192,7 @@ func downloadLatestBinary(tc *TestConfig) (string, error) {
192192
if err != nil {
193193
if !errors.Is(err, errStoppedInMiddle) {
194194
if err := os.Remove(localFile); err != nil {
195-
log.Printf("failed to remove %s: %s", localFile, err)
195+
log.Printf("failed to remove %s: %v", localFile, err)
196196
}
197197
}
198198
return "", err

testserver/tenant.go

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func (ts *testServerImpl) isTenant() bool {
5050
// NewTestServer to this interface. Refer to the tests for an example.
5151
func (ts *testServerImpl) NewTenantServer(proxy bool) (TestServer, error) {
5252
if proxy && !ts.serverArgs.secure {
53-
return nil, errors.New("proxy can not be used with insecure mode")
53+
return nil, fmt.Errorf("%s: proxy cannot be used with insecure mode", tenantserverMessagePrefix)
5454
}
5555
cockroachBinary := ts.cmdArgs[0]
5656
tenantID, err := func() (int, error) {
@@ -67,7 +67,7 @@ func (ts *testServerImpl) NewTenantServer(proxy bool) (TestServer, error) {
6767
return tenantID, nil
6868
}()
6969
if err != nil {
70-
return nil, err
70+
return nil, fmt.Errorf("%s: %w", tenantserverMessagePrefix, err)
7171
}
7272

7373
secureFlag := "--insecure"
@@ -83,25 +83,25 @@ func (ts *testServerImpl) NewTenantServer(proxy bool) (TestServer, error) {
8383
{"mt", "cert", "create-tenant-client", fmt.Sprint(tenantID)},
8484
} {
8585
if err := exec.Command(cockroachBinary, append(args, certArgs...)...).Run(); err != nil {
86-
return nil, err
86+
return nil, fmt.Errorf("%s: %w", tenantserverMessagePrefix, err)
8787
}
8888
}
8989
}
9090
// Create a new tenant.
9191
if err := ts.WaitForInit(); err != nil {
92-
return nil, err
92+
return nil, fmt.Errorf("%s: %w", tenantserverMessagePrefix, err)
9393
}
9494
pgURL := ts.PGURL()
9595
if pgURL == nil {
96-
return nil, errors.New("url not found")
96+
return nil, fmt.Errorf("%s: url not found", tenantserverMessagePrefix)
9797
}
9898
db, err := sql.Open("postgres", pgURL.String())
9999
if err != nil {
100-
return nil, err
100+
return nil, fmt.Errorf("%s: %w", tenantserverMessagePrefix, err)
101101
}
102102
defer db.Close()
103103
if _, err := db.Exec(fmt.Sprintf("SELECT crdb_internal.create_tenant(%d)", tenantID)); err != nil {
104-
return nil, err
104+
return nil, fmt.Errorf("%s: %w", tenantserverMessagePrefix, err)
105105
}
106106

107107
// TODO(asubiotto): We should pass ":0" as the sql addr to push port
@@ -112,13 +112,13 @@ func (ts *testServerImpl) NewTenantServer(proxy bool) (TestServer, error) {
112112
addr := func() (string, error) {
113113
l, err := net.Listen("tcp", ":0")
114114
if err != nil {
115-
return "", err
115+
return "", fmt.Errorf("%s: %w", tenantserverMessagePrefix, err)
116116
}
117117
// Use localhost because of certificate validation issues otherwise
118118
// (something about IP SANs).
119119
addr := "localhost:" + strconv.Itoa(l.Addr().(*net.TCPAddr).Port)
120120
if err := l.Close(); err != nil {
121-
return "", err
121+
return "", fmt.Errorf("%s: %w", tenantserverMessagePrefix, err)
122122
}
123123
return addr, nil
124124
}
@@ -157,17 +157,18 @@ func (ts *testServerImpl) NewTenantServer(proxy bool) (TestServer, error) {
157157
}
158158
cmd := exec.Command(cockroachBinary, args...)
159159
if err := cmd.Start(); err != nil {
160-
return "", err
160+
return "", fmt.Errorf("%s: %w", tenantserverMessagePrefix, err)
161161
}
162162
if cmd.Process != nil {
163-
log.Printf("process %d started: %s", cmd.Process.Pid, strings.Join(args, " "))
163+
log.Printf("%s: process %d started: %s", tenantserverMessagePrefix, cmd.Process.Pid,
164+
strings.Join(args, " "))
164165
}
165166
ts.proxyProcess = cmd.Process
166167

167168
return ts.proxyAddr, nil
168169
}()
169170
if err != nil {
170-
return nil, err
171+
return nil, fmt.Errorf("%s: %w", tenantserverMessagePrefix, err)
171172
}
172173

173174
args := []string{
@@ -203,15 +204,15 @@ func (ts *testServerImpl) NewTenantServer(proxy bool) (TestServer, error) {
203204

204205
tenant.setPGURL(&tenantURL)
205206
if err := tenant.Start(); err != nil {
206-
return nil, err
207+
return nil, fmt.Errorf("%s: %w", tenantserverMessagePrefix, err)
207208
}
208209
if err := tenant.WaitForInit(); err != nil {
209-
return nil, err
210+
return nil, fmt.Errorf("%s: %w", tenantserverMessagePrefix, err)
210211
}
211212

212213
tenantDB, err := sql.Open("postgres", tenantURL.String())
213214
if err != nil {
214-
return nil, err
215+
return nil, fmt.Errorf("%s: %w", tenantserverMessagePrefix, err)
215216
}
216217
defer tenantDB.Close()
217218

@@ -227,7 +228,7 @@ func (ts *testServerImpl) NewTenantServer(proxy bool) (TestServer, error) {
227228
if rootPassword != "" {
228229
// Allow root to login via password.
229230
if _, err := tenantDB.Exec(`ALTER USER $1 WITH PASSWORD $2`, "root", rootPassword); err != nil {
230-
return nil, err
231+
return nil, fmt.Errorf("%s: %w", tenantserverMessagePrefix, err)
231232
}
232233

233234
// NB: need the lock since *tenantURL is owned by `tenant`.

testserver/testserver.go

Lines changed: 40 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,9 @@ const (
7979
// By default, we allocate 20% of available memory to the test server.
8080
const defaultStoreMemSize = 0.2
8181

82+
const testserverMessagePrefix = "cockroach-go testserver"
83+
const tenantserverMessagePrefix = "cockroach-go tenantserver"
84+
8285
// TestServer is a helper to run a real cockroach node.
8386
type TestServer interface {
8487
// Start starts the server.
@@ -164,7 +167,7 @@ func NewDBForTestWithDatabase(
164167

165168
db, err := sql.Open("postgres", url.String())
166169
if err != nil {
167-
t.Fatal(err)
170+
t.Fatalf("%s: %v", testserverMessagePrefix, err)
168171
}
169172

170173
return db, func() {
@@ -276,7 +279,7 @@ func NewTestServer(opts ...TestServerOpt) (TestServer, error) {
276279
// return error.
277280
return nil, err
278281
}
279-
log.Printf("Failed to fetch latest binary: %s, attempting to use cockroach binary from your PATH", err)
282+
log.Printf("%s: Failed to fetch latest binary: %v attempting to use cockroach binary from your PATH", testserverMessagePrefix, err)
280283
cockroachBinary = "cockroach"
281284
} else {
282285
log.Printf("Using automatically-downloaded binary: %s", cockroachBinary)
@@ -287,13 +290,15 @@ func NewTestServer(opts ...TestServerOpt) (TestServer, error) {
287290
// which get us over the socket filename length limit.
288291
baseDir, err := ioutil.TempDir("/tmp", "cockroach-testserver")
289292
if err != nil {
290-
return nil, fmt.Errorf("could not create temp directory: %s", err)
293+
return nil, fmt.Errorf("%s: could not create temp directory: %w",
294+
testserverMessagePrefix, err)
291295
}
292296

293297
mkDir := func(name string) (string, error) {
294298
path := filepath.Join(baseDir, name)
295299
if err := os.MkdirAll(path, 0755); err != nil {
296-
return "", fmt.Errorf("could not create %s directory: %s: %s", name, path, err)
300+
return "", fmt.Errorf("%s: could not create %s directory: %s: %w",
301+
testserverMessagePrefix, name, path, err)
297302
}
298303
return path, nil
299304
}
@@ -302,11 +307,11 @@ func NewTestServer(opts ...TestServerOpt) (TestServer, error) {
302307
// https://github.com/cockroachdb/cockroach-go/issues/109
303308
logDir, err := mkDir(logsDirName)
304309
if err != nil {
305-
return nil, err
310+
return nil, fmt.Errorf("%s: %w", testserverMessagePrefix, err)
306311
}
307312
certsDir, err := mkDir(certsDirName)
308313
if err != nil {
309-
return nil, err
314+
return nil, fmt.Errorf("%s: %w", testserverMessagePrefix, err)
310315
}
311316

312317
listeningURLFile := filepath.Join(baseDir, "listen-url")
@@ -327,7 +332,7 @@ func NewTestServer(opts ...TestServerOpt) (TestServer, error) {
327332
{"cert", "create-client", "root"},
328333
} {
329334
if err := exec.Command(cockroachBinary, append(args, certArgs...)...).Run(); err != nil {
330-
return nil, err
335+
return nil, fmt.Errorf("%s: %w", testserverMessagePrefix, err)
331336
}
332337
}
333338
secureOpt = "--certs-dir=" + certsDir
@@ -339,17 +344,17 @@ func NewTestServer(opts ...TestServerOpt) (TestServer, error) {
339344
versionCmd := exec.Command(cockroachBinary, "version")
340345
versionOutput, err := versionCmd.CombinedOutput()
341346
if err != nil {
342-
return nil, err
347+
return nil, fmt.Errorf("%s: %w", testserverMessagePrefix, err)
343348
}
344349
reader := bufio.NewReader(bytes.NewReader(versionOutput))
345350
versionLine, err := reader.ReadString('\n')
346351
if err != nil {
347-
return nil, err
352+
return nil, fmt.Errorf("%s: %w", testserverMessagePrefix, err)
348353
}
349354
versionLineTokens := strings.Fields(versionLine)
350355
v, err := version.Parse(versionLineTokens[2])
351356
if err != nil {
352-
return nil, err
357+
return nil, fmt.Errorf("%s: %w", testserverMessagePrefix, err)
353358
}
354359
startCmd := "start-single-node"
355360
if !v.AtLeast(version.MustParse("v19.2.0-alpha")) {
@@ -388,15 +393,15 @@ func NewTestServer(opts ...TestServerOpt) (TestServer, error) {
388393
ts.pgURL.set = make(chan struct{})
389394

390395
if err := ts.Start(); err != nil {
391-
return nil, err
396+
return nil, fmt.Errorf("%s: %w", testserverMessagePrefix, err)
392397
}
393398

394399
if ts.PGURL() == nil {
395-
return nil, errors.New("testserver: url not found")
400+
return nil, fmt.Errorf("%s: url not found", testserverMessagePrefix)
396401
}
397402

398403
if err := ts.WaitForInit(); err != nil {
399-
return nil, err
404+
return nil, fmt.Errorf("%s: %w", testserverMessagePrefix, err)
400405
}
401406

402407
return ts, nil
@@ -439,7 +444,7 @@ func (ts *testServerImpl) WaitForInit() error {
439444
if _, err = db.Query("SHOW DATABASES"); err == nil {
440445
return err
441446
}
442-
log.Printf("WaitForInit: Trying again after error: %v", err)
447+
log.Printf("%s: WaitForInit: Trying again after error: %v", testserverMessagePrefix, err)
443448
time.Sleep(time.Millisecond * 100)
444449
}
445450
return err
@@ -460,14 +465,14 @@ func (ts *testServerImpl) pollListeningURLFile() error {
460465
if err == nil {
461466
break
462467
} else if !os.IsNotExist(err) {
463-
return fmt.Errorf("unexpected error while reading listening URL file: %v", err)
468+
return fmt.Errorf("unexpected error while reading listening URL file: %w", err)
464469
}
465470
time.Sleep(100 * time.Millisecond)
466471
}
467472

468473
u, err := url.Parse(string(bytes.TrimSpace(data)))
469474
if err != nil {
470-
return fmt.Errorf("failed to parse SQL URL: %v", err)
475+
return fmt.Errorf("failed to parse SQL URL: %w", err)
471476
}
472477
ts.pgURL.orig = *u
473478
if pw := ts.serverArgs.rootPW; pw != "" {
@@ -507,7 +512,7 @@ func (ts *testServerImpl) Start() error {
507512
case stateStopped, stateFailed:
508513
// Start() can only be called once.
509514
return errors.New(
510-
"Start() cannot be used to restart a stopped or failed server. " +
515+
"`Start()` cannot be used to restart a stopped or failed server. " +
511516
"Please use NewTestServer()")
512517
}
513518
}
@@ -523,7 +528,7 @@ func (ts *testServerImpl) Start() error {
523528
if len(ts.stdout) > 0 {
524529
wr, err := newFileLogWriter(ts.stdout)
525530
if err != nil {
526-
return fmt.Errorf("unable to open file %s: %s", ts.stdout, err)
531+
return fmt.Errorf("unable to open file %s: %w", ts.stdout, err)
527532
}
528533
ts.stdoutBuf = wr
529534
}
@@ -532,7 +537,7 @@ func (ts *testServerImpl) Start() error {
532537
if len(ts.stderr) > 0 {
533538
wr, err := newFileLogWriter(ts.stderr)
534539
if err != nil {
535-
return fmt.Errorf("unable to open file %s: %s", ts.stderr, err)
540+
return fmt.Errorf("unable to open file %s: %w", ts.stderr, err)
536541
}
537542
ts.stderrBuf = wr
538543
}
@@ -549,33 +554,37 @@ func (ts *testServerImpl) Start() error {
549554
if err != nil {
550555
log.Print(err.Error())
551556
if err := ts.stdoutBuf.Close(); err != nil {
552-
log.Printf("failed to close stdout: %s", err)
557+
log.Printf("%s: failed to close stdout: %v", testserverMessagePrefix, err)
553558
}
554559
if err := ts.stderrBuf.Close(); err != nil {
555-
log.Printf("failed to close stderr: %s", err)
560+
log.Printf("%s: failed to close stderr: %v", testserverMessagePrefix, err)
556561
}
557562

558563
ts.mu.Lock()
559564
ts.state = stateFailed
560565
ts.mu.Unlock()
561566

562-
return fmt.Errorf("failure starting process: %s", err)
567+
return fmt.Errorf("failure starting process: %w", err)
563568
}
564569

565570
go func() {
566571
err := ts.cmd.Wait()
567572

568573
if err := ts.stdoutBuf.Close(); err != nil {
569-
log.Printf("failed to close stdout: %s", err)
574+
log.Printf("%s: failed to close stdout: %v", testserverMessagePrefix, err)
570575
}
571576
if err := ts.stderrBuf.Close(); err != nil {
572-
log.Printf("failed to close stderr: %s", err)
577+
log.Printf("%s: failed to close stderr: %v", testserverMessagePrefix, err)
573578
}
574579

575580
ps := ts.cmd.ProcessState
576581
sy := ps.Sys().(syscall.WaitStatus)
577582

578-
log.Printf("Process %d exited with status %d: %v", ps.Pid(), sy.ExitStatus(), err)
583+
log.Printf("%s: Process %d exited with status %d: %v",
584+
testserverMessagePrefix,
585+
ps.Pid(),
586+
sy.ExitStatus(),
587+
err)
579588
log.Print(ps.String())
580589

581590
ts.mu.Lock()
@@ -590,7 +599,7 @@ func (ts *testServerImpl) Start() error {
590599
if ts.pgURL.u == nil {
591600
go func() {
592601
if err := ts.pollListeningURLFile(); err != nil {
593-
log.Printf("%v", err)
602+
log.Printf("%s: %v", testserverMessagePrefix, err)
594603
close(ts.pgURL.set)
595604
ts.Stop()
596605
}
@@ -608,11 +617,13 @@ func (ts *testServerImpl) Stop() {
608617
defer ts.mu.RUnlock()
609618

610619
if ts.state == stateNew {
611-
log.Fatal("Stop() called, but Start() was never called")
620+
log.Fatalf("%s: Stop() called, but Start() was never called", testserverMessagePrefix)
612621
}
613622
if ts.state == stateFailed {
614-
log.Fatalf("Stop() called, but process exited unexpectedly. Stdout:\n%s\nStderr:\n%s\n",
615-
ts.Stdout(), ts.Stderr())
623+
log.Fatalf("%s: Stop() called, but process exited unexpectedly. Stdout:\n%s\nStderr:\n%s\n",
624+
testserverMessagePrefix,
625+
ts.Stdout(),
626+
ts.Stderr())
616627
return
617628
}
618629

0 commit comments

Comments
 (0)