fix(download-clients): qbit nil logger panic (#344)

The logger wasn't set, so it was always nil. Change to always initialize and override if one is passed.

internal/action/qbittorrent.go

@@ -6,6 +6,9 @@ import (
 	"strings"
 	"time"
 
+	"github.com/dcarbone/zadapters/zstdlog"
+	"github.com/rs/zerolog"
+
 	"github.com/autobrr/autobrr/internal/domain"
 	"github.com/autobrr/autobrr/pkg/errors"
 	"github.com/autobrr/autobrr/pkg/qbittorrent"
@@ -30,15 +33,18 @@ func (s *service) qbittorrent(action domain.Action, release domain.Release) ([]s
 	qbt, exists := s.qbitClients[qbitKey{client.ID, client.Name}]
 	if !exists {
 		qbtSettings := qbittorrent.Settings{
+			Name:          client.Name,
 			Hostname:      client.Host,
 			Port:          uint(client.Port),
 			Username:      client.Username,
 			Password:      client.Password,
 			TLS:           client.TLS,
 			TLSSkipVerify: client.TLSSkipVerify,
-			Log:           s.subLogger,
 		}
 
+		// setup sub logger adapter which is compatible with *log.Logger
+		qbtSettings.Log = zstdlog.NewStdLoggerWithLevel(s.log.With().Str("type", "qBittorrent").Str("client", client.Name).Logger(), zerolog.TraceLevel)
+
 		// only set basic auth if enabled
 		if client.Settings.Basic.Auth {
 			qbtSettings.BasicAuth = client.Settings.Basic.Auth
@@ -47,7 +53,6 @@ func (s *service) qbittorrent(action domain.Action, release domain.Release) ([]s
 		}
 
 		qbt = qbittorrent.NewClient(qbtSettings)
-		qbt.Name = client.Name
 
 		s.qbitClients[qbitKey{client.ID, client.Name}] = qbt
 
@@ -87,7 +92,7 @@ func (s *service) qbittorrent(action domain.Action, release domain.Release) ([]s
 	s.log.Trace().Msgf("action qBittorrent options: %+v", options)
 
 	if err = qbt.AddTorrentFromFile(release.TorrentTmpFile, options); err != nil {
-		return nil, errors.Wrap(err, "could not add torrent %v to client: %v", release.TorrentTmpFile, qbt.Name)
+		return nil, errors.Wrap(err, "could not add torrent %v to client: %v", release.TorrentTmpFile, client.Name)
 	}
 
 	if !action.Paused && !action.ReAnnounceSkip && release.TorrentHash != "" {
@@ -96,7 +101,7 @@ func (s *service) qbittorrent(action domain.Action, release domain.Release) ([]s
 		}
 	}
 
-	s.log.Info().Msgf("torrent with hash %v successfully added to client: '%v'", release.TorrentHash, qbt.Name)
+	s.log.Info().Msgf("torrent with hash %v successfully added to client: '%v'", release.TorrentHash, client.Name)
 
 	return nil, nil
 }
@@ -152,7 +157,7 @@ func (s *service) prepareQbitOptions(action domain.Action, m Macro) (map[string]
 	return options, nil
 }
 
-func (s *service) qbittorrentCheckRulesCanDownload(action domain.Action, client *domain.DownloadClient, qbt *qbittorrent.Client) ([]string, error) {
+func (s *service) qbittorrentCheckRulesCanDownload(action domain.Action, client *domain.DownloadClient, qbt qbittorrent.Client) ([]string, error) {
 	s.log.Trace().Msgf("action qBittorrent: %v check rules", action.Name)
 
 	// check for active downloads and other rules
@@ -197,7 +202,7 @@ func (s *service) qbittorrentCheckRulesCanDownload(action domain.Action, client
 	return nil, nil
 }
 
-func (s *service) reannounceTorrent(qb *qbittorrent.Client, action domain.Action, hash string) error {
+func (s *service) reannounceTorrent(qb qbittorrent.Client, action domain.Action, hash string) error {
 	announceOK := false
 	attempts := 0
 

internal/action/run.go

@@ -20,6 +20,14 @@ func (s *service) RunAction(action *domain.Action, release domain.Release) ([]st
 		rejections []string
 	)
 
+	defer func() {
+		if r := recover(); r != nil {
+			s.log.Error().Msgf("recovering from panic in run action %v error: %v", action.Name, r)
+			err = errors.New("panic in action: %v", action.Name)
+			return
+		}
+	}()
+
 	switch action.Type {
 	case domain.ActionTypeTest:
 		s.test(action.Name)

internal/action/service.go

@@ -36,7 +36,7 @@ type service struct {
 	clientSvc download_client.Service
 	bus       EventBus.Bus
 
-	qbitClients map[qbitKey]*qbittorrent.Client
+	qbitClients map[qbitKey]qbittorrent.Client
 }
 
 func NewService(log logger.Logger, repo domain.ActionRepo, clientSvc download_client.Service, bus EventBus.Bus) Service {
@@ -45,7 +45,7 @@ func NewService(log logger.Logger, repo domain.ActionRepo, clientSvc download_cl
 		repo:        repo,
 		clientSvc:   clientSvc,
 		bus:         bus,
-		qbitClients: map[qbitKey]*qbittorrent.Client{},
+		qbitClients: map[qbitKey]qbittorrent.Client{},
 	}
 
 	s.subLogger = zstdlog.NewStdLoggerWithLevel(s.log.With().Logger(), zerolog.TraceLevel)

pkg/qbittorrent/client.go

@@ -15,11 +15,24 @@ import (
 	"strings"
 	"time"
 
-	"github.com/autobrr/autobrr/pkg/errors"
+	"golang.org/x/net/publicsuffix"
 
-	publicsuffix "golang.org/x/net/publicsuffix"
+	"github.com/autobrr/autobrr/pkg/errors"
 )
 
+type Client interface {
+	Login() error
+	GetTorrents() ([]Torrent, error)
+	GetTorrentsFilter(filter TorrentFilter) ([]Torrent, error)
+	GetTorrentsActiveDownloads() ([]Torrent, error)
+	GetTorrentsRaw() (string, error)
+	GetTorrentTrackers(hash string) ([]TorrentTracker, error)
+	AddTorrentFromFile(file string, options map[string]string) error
+	DeleteTorrents(hashes []string, deleteFiles bool) error
+	ReAnnounceTorrents(hashes []string) error
+	GetTransferInfo() (*TransferInfo, error)
+}
+
 var (
 	backoffSchedule = []time.Duration{
 		5 * time.Second,
@@ -29,15 +42,16 @@ var (
 	timeout = 60 * time.Second
 )
 
-type Client struct {
+type client struct {
 	Name     string
 	settings Settings
 	http     *http.Client
 
-	Log *log.Logger
+	log *log.Logger
 }
 
 type Settings struct {
+	Name          string
 	Hostname      string
 	Port          uint
 	Username      string
@@ -55,20 +69,23 @@ type Basic struct {
 	Password string
 }
 
-func NewClient(s Settings) *Client {
+func NewClient(settings Settings) Client {
-	c := &Client{
+	c := &client{
-		settings: s,
+		settings: settings,
+		Name:     settings.Name,
+		log:      log.New(io.Discard, "", log.LstdFlags),
 	}
 
-	if s.Log == nil {
+	// override logger if we pass one
-		c.Log = log.New(io.Discard, "qbittorrent", log.LstdFlags)
+	if settings.Log != nil {
+		c.log = settings.Log
 	}
 
 	//store cookies in jar
 	jarOptions := &cookiejar.Options{PublicSuffixList: publicsuffix.List}
 	jar, err := cookiejar.New(jarOptions)
 	if err != nil {
-		c.Log.Println("new client cookie error")
+		c.log.Println("new client cookie error")
 	}
 
 	c.http = &http.Client{
@@ -93,7 +110,7 @@ func NewClient(s Settings) *Client {
 	return c
 }
 
-func (c *Client) get(endpoint string, opts map[string]string) (*http.Response, error) {
+func (c *client) get(endpoint string, opts map[string]string) (*http.Response, error) {
 	var err error
 	var resp *http.Response
 
@@ -117,7 +134,7 @@ func (c *Client) get(endpoint string, opts map[string]string) (*http.Response, e
 			break
 		}
 
-		c.Log.Printf("qbit GET failed: retrying attempt %d - %v\n", i, reqUrl)
+		c.log.Printf("qbit GET failed: retrying attempt %d - %v\n", i, reqUrl)
 
 		time.Sleep(backoff)
 	}
@@ -129,7 +146,7 @@ func (c *Client) get(endpoint string, opts map[string]string) (*http.Response, e
 	return resp, nil
 }
 
-func (c *Client) post(endpoint string, opts map[string]string) (*http.Response, error) {
+func (c *client) post(endpoint string, opts map[string]string) (*http.Response, error) {
 	// add optional parameters that the user wants
 	form := url.Values{}
 	if opts != nil {
@@ -164,7 +181,7 @@ func (c *Client) post(endpoint string, opts map[string]string) (*http.Response,
 			break
 		}
 
-		c.Log.Printf("qbit POST failed: retrying attempt %d - %v\n", i, reqUrl)
+		c.log.Printf("qbit POST failed: retrying attempt %d - %v\n", i, reqUrl)
 
 		time.Sleep(backoff)
 	}
@@ -176,7 +193,7 @@ func (c *Client) post(endpoint string, opts map[string]string) (*http.Response,
 	return resp, nil
 }
 
-func (c *Client) postBasic(endpoint string, opts map[string]string) (*http.Response, error) {
+func (c *client) postBasic(endpoint string, opts map[string]string) (*http.Response, error) {
 	// add optional parameters that the user wants
 	form := url.Values{}
 	if opts != nil {
@@ -210,7 +227,7 @@ func (c *Client) postBasic(endpoint string, opts map[string]string) (*http.Respo
 	return resp, nil
 }
 
-func (c *Client) postFile(endpoint string, fileName string, opts map[string]string) (*http.Response, error) {
+func (c *client) postFile(endpoint string, fileName string, opts map[string]string) (*http.Response, error) {
 	var err error
 	var resp *http.Response
 
@@ -279,7 +296,7 @@ func (c *Client) postFile(endpoint string, fileName string, opts map[string]stri
 			break
 		}
 
-		c.Log.Printf("qbit POST file failed: retrying attempt %d - %v\n", i, reqUrl)
+		c.log.Printf("qbit POST file failed: retrying attempt %d - %v\n", i, reqUrl)
 
 		time.Sleep(backoff)
 	}
@@ -291,7 +308,7 @@ func (c *Client) postFile(endpoint string, fileName string, opts map[string]stri
 	return resp, nil
 }
 
-func (c *Client) setCookies(cookies []*http.Cookie) {
+func (c *client) setCookies(cookies []*http.Cookie) {
 	cookieURL, _ := url.Parse(buildUrl(c.settings, ""))
 
 	c.http.Jar.SetCookies(cookieURL, cookies)

pkg/qbittorrent/methods.go

@@ -12,7 +12,7 @@ import (
 )
 
 // Login https://github.com/qbittorrent/qBittorrent/wiki/WebUI-API-(qBittorrent-4.1)#authentication
-func (c *Client) Login() error {
+func (c *client) Login() error {
 	opts := map[string]string{
 		"username": c.settings.Username,
 		"password": c.settings.Password,
@@ -50,10 +50,12 @@ func (c *Client) Login() error {
 		return errors.New("bad credentials")
 	}
 
+	c.log.Printf("logged into client: %v", c.Name)
+
 	return nil
 }
 
-func (c *Client) GetTorrents() ([]Torrent, error) {
+func (c *client) GetTorrents() ([]Torrent, error) {
 
 	resp, err := c.get("torrents/info", nil)
 	if err != nil {
@@ -76,7 +78,7 @@ func (c *Client) GetTorrents() ([]Torrent, error) {
 	return torrents, nil
 }
 
-func (c *Client) GetTorrentsFilter(filter TorrentFilter) ([]Torrent, error) {
+func (c *client) GetTorrentsFilter(filter TorrentFilter) ([]Torrent, error) {
 	opts := map[string]string{
 		"filter": string(filter),
 	}
@@ -102,7 +104,7 @@ func (c *Client) GetTorrentsFilter(filter TorrentFilter) ([]Torrent, error) {
 	return torrents, nil
 }
 
-func (c *Client) GetTorrentsActiveDownloads() ([]Torrent, error) {
+func (c *client) GetTorrentsActiveDownloads() ([]Torrent, error) {
 	var filter = TorrentFilterDownloading
 
 	opts := map[string]string{
@@ -139,7 +141,7 @@ func (c *Client) GetTorrentsActiveDownloads() ([]Torrent, error) {
 	return res, nil
 }
 
-func (c *Client) GetTorrentsRaw() (string, error) {
+func (c *client) GetTorrentsRaw() (string, error) {
 	resp, err := c.get("torrents/info", nil)
 	if err != nil {
 		return "", errors.Wrap(err, "could not get torrents raw")
@@ -155,7 +157,7 @@ func (c *Client) GetTorrentsRaw() (string, error) {
 	return string(data), nil
 }
 
-func (c *Client) GetTorrentTrackers(hash string) ([]TorrentTracker, error) {
+func (c *client) GetTorrentTrackers(hash string) ([]TorrentTracker, error) {
 	opts := map[string]string{
 		"hash": hash,
 	}
@@ -169,10 +171,11 @@ func (c *Client) GetTorrentTrackers(hash string) ([]TorrentTracker, error) {
 
 	dump, err := httputil.DumpResponse(resp, true)
 	if err != nil {
-		c.Log.Printf("get torrent trackers error dump response: %v\n", string(dump))
+		//c.log.Printf("get torrent trackers error dump response: %v\n", string(dump))
+		return nil, errors.Wrap(err, "could not dump response for hash: %v", hash)
 	}
 
-	c.Log.Printf("get torrent trackers response dump: %v\n", string(dump))
+	c.log.Printf("get torrent trackers response dump: %q", dump)
 
 	if resp.StatusCode == http.StatusNotFound {
 		return nil, nil
@@ -185,7 +188,7 @@ func (c *Client) GetTorrentTrackers(hash string) ([]TorrentTracker, error) {
 		return nil, errors.Wrap(err, "could not read body")
 	}
 
-	c.Log.Printf("get torrent trackers body: %v\n", string(body))
+	c.log.Printf("get torrent trackers body: %v\n", string(body))
 
 	var trackers []TorrentTracker
 	err = json.Unmarshal(body, &trackers)
@@ -197,7 +200,7 @@ func (c *Client) GetTorrentTrackers(hash string) ([]TorrentTracker, error) {
 }
 
 // AddTorrentFromFile add new torrent from torrent file
-func (c *Client) AddTorrentFromFile(file string, options map[string]string) error {
+func (c *client) AddTorrentFromFile(file string, options map[string]string) error {
 
 	res, err := c.postFile("torrents/add", file, options)
 	if err != nil {
@@ -211,7 +214,7 @@ func (c *Client) AddTorrentFromFile(file string, options map[string]string) erro
 	return nil
 }
 
-func (c *Client) DeleteTorrents(hashes []string, deleteFiles bool) error {
+func (c *client) DeleteTorrents(hashes []string, deleteFiles bool) error {
 	// Add hashes together with | separator
 	hv := strings.Join(hashes, "|")
 
@@ -232,7 +235,7 @@ func (c *Client) DeleteTorrents(hashes []string, deleteFiles bool) error {
 	return nil
 }
 
-func (c *Client) ReAnnounceTorrents(hashes []string) error {
+func (c *client) ReAnnounceTorrents(hashes []string) error {
 	// Add hashes together with | separator
 	hv := strings.Join(hashes, "|")
 	opts := map[string]string{
@@ -251,7 +254,7 @@ func (c *Client) ReAnnounceTorrents(hashes []string) error {
 	return nil
 }
 
-func (c *Client) GetTransferInfo() (*TransferInfo, error) {
+func (c *client) GetTransferInfo() (*TransferInfo, error) {
 	resp, err := c.get("transfer/info", nil)
 	if err != nil {
 		return nil, errors.Wrap(err, "could not get transfer info")