Skip to content

Commit 5185956

Browse files
authored
fix: don't panic when logs waits for more than 5 seconds (testcontainers#947)
This removes panic when logs endpoint takes more than 5 seconds to respond. The panic happened at least with podman when no new logs appear when using follow and since parameters. We keep retrying until the context is canceled (the retry request would fail anyway with canceled context) or the producer is stopped, whichever comes first. This makes the retry behavior consistent with closed connections handling. This should fix testcontainers#946
1 parent 82e1d12 commit 5185956

File tree

1 file changed

+15
-12
lines changed

1 file changed

+15
-12
lines changed

docker.go

+15-12
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ type DockerContainer struct {
6969
terminationSignal chan bool
7070
consumers []LogConsumer
7171
raw *types.ContainerJSON
72-
stopProducer chan bool
72+
stopProducer context.CancelFunc
7373
logger Logging
7474
lifecycleHooks []ContainerLifecycleHooks
7575
}
@@ -632,9 +632,9 @@ func (c *DockerContainer) StartLogProducer(ctx context.Context) error {
632632
return errors.New("log producer already started")
633633
}
634634

635-
c.stopProducer = make(chan bool)
635+
ctx, c.stopProducer = context.WithCancel(ctx)
636636

637-
go func(stop <-chan bool) {
637+
go func() {
638638
since := ""
639639
// if the socket is closed we will make additional logs request with updated Since timestamp
640640
BEGIN:
@@ -645,20 +645,22 @@ func (c *DockerContainer) StartLogProducer(ctx context.Context) error {
645645
Since: since,
646646
}
647647

648-
ctx, cancel := context.WithTimeout(ctx, time.Second*5)
649-
defer cancel()
650-
651648
r, err := c.provider.client.ContainerLogs(ctx, c.GetContainerID(), options)
652649
if err != nil {
653-
// if we can't get the logs, panic, we can't return an error to anything
654-
// from within this goroutine
655-
panic(err)
650+
// if we can't get the logs, retry in one second.
651+
c.logger.Printf("cannot get logs for container %q: %v", c.ID, err)
652+
if ctx.Err() != nil {
653+
// context done.
654+
return
655+
}
656+
time.Sleep(1 * time.Second)
657+
goto BEGIN
656658
}
657659
defer c.provider.Close()
658660

659661
for {
660662
select {
661-
case <-stop:
663+
case <-ctx.Done():
662664
err := r.Close()
663665
if err != nil {
664666
// we can't close the read closer, this should never happen
@@ -709,7 +711,7 @@ func (c *DockerContainer) StartLogProducer(ctx context.Context) error {
709711
}
710712
}
711713
}
712-
}(c.stopProducer)
714+
}()
713715

714716
return nil
715717
}
@@ -718,7 +720,8 @@ func (c *DockerContainer) StartLogProducer(ctx context.Context) error {
718720
// and sending them to each added LogConsumer
719721
func (c *DockerContainer) StopLogProducer() error {
720722
if c.stopProducer != nil {
721-
c.stopProducer <- true
723+
// Cancel the producer's context.
724+
c.stopProducer()
722725
c.stopProducer = nil
723726
}
724727
return nil

0 commit comments

Comments
 (0)