Repository: derailed/k9s
File: internal/dao/pod.go (Go)

Code before:

			// Read logs until stream ends or errors occur
			streamDone := make(chan bool, 1)
			wg.Add(1)
			go func() {
				defer wg.Done()
				readLogs(ctx, stream, out, opts, streamDone)
			}()

			// Wait for stream to finish or context cancellation
			select {
			case <-ctx.Done():
				stream.Close()
				return
			case <-streamDone:
				// Stream ended (possibly due to error), retry connection
				slog.Debug("Log stream ended, retrying connection",
					slogs.Container, opts.Info(),
				)
				time.Sleep(logRetryWait)
				continue
			}
		}

		// If we've exhausted all retries, send final error
		out <- opts.ToErrLogItem(fmt.Errorf("failed to maintain log stream after %d retries", logRetryCount))
	}()
	go func() {
		wg.Wait()
		close(out)
	}()

	return out
}

func readLogs(ctx context.Context, stream io.ReadCloser, out chan<- *LogItem, opts *LogOptions, streamDone chan<- bool) {
	defer func() {
		if err := stream.Close(); err != nil {
			slog.Error("Fail to close stream",
				slogs.Container, opts.Info(),
				slogs.Error, err,
			)
		}
		// Signal that the stream has ended
		select {
		case streamDone <- true:
		default:
		}
	}()

	slog.Debug("Processing logs", slogs.Options, opts.Info())

Review comment (security):
in both case the log item is created but never used... There is no way for the call site to distinguish EOF vs stream error. It's not clear in this PR what happens to the stream when the credentials expired? Is it closed or does it error out??
Also streamDone chan does not provide any real convenience. Better to know if this warrants a retry or we need to bail out. Thus we can't determine a legit close ie init container vs main container crapping out.
We need to be very careful here since we are in a tight loop and firing off gors. 
The call site needs to get a clear indication of what action to take next aka cancel, retry or bail out.
IMHO only stream errors should warrant retries.

Code after:
			// Check if we should stop retrying based on pod status
			if pod, ok := logger.(*Pod); ok && pod.shouldStopRetrying(opts.Path) {
				slog.Debug("Stopping log retry - pod is terminating or deleted",
					slogs.Container, opts.Info(),
				)
				return
			}

			req, err := logger.Logs(opts.Path, podOpts)
			if err != nil {
				slog.Error("Log request failed",
					slogs.Container, opts.Info(),
					slogs.Error, err,
				)
				select {
				case <-ctx.Done():
					return
				case <-time.After(delay):
					if delay = backoffCtx.NextBackOff(); delay == backoff.Stop {
						return
					}
				}
				continue
			}

			stream, e := req.Stream(ctx)
			if e != nil {
				slog.Error("Stream logs failed",
					slogs.Error, e,
					slogs.Container, opts.Info(),
				)
				select {
				case <-ctx.Done():
					return
				case <-time.After(delay):
					if delay = backoffCtx.NextBackOff(); delay == backoff.Stop {
						return
					}
				}
				continue
			}

			// Process logs until completion
			result := readLogs(ctx, stream, out, opts)
			switch result {
			case streamEOF:
				slog.Debug("Log stream ended cleanly",
					slogs.Container, opts.Info(),
				)
				return
			case streamError: