From 401c93a65763f05f3e4d993cd6c9ee58a04f80a5 Mon Sep 17 00:00:00 2001 From: Ludvig Lundgren Date: Fri, 8 Jul 2022 22:12:19 +0200 Subject: [PATCH] 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 | 17 +++++++---- internal/action/run.go | 8 +++++ internal/action/service.go | 4 +-- pkg/qbittorrent/client.go | 53 ++++++++++++++++++++++------------ pkg/qbittorrent/methods.go | 29 ++++++++++--------- 5 files changed, 72 insertions(+), 39 deletions(-) diff --git a/internal/action/qbittorrent.go b/internal/action/qbittorrent.go index 86588da..b091800 100644 --- a/internal/action/qbittorrent.go +++ b/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 diff --git a/internal/action/run.go b/internal/action/run.go index 65d25ca..1261f46 100644 --- a/internal/action/run.go +++ b/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) diff --git a/internal/action/service.go b/internal/action/service.go index dd8d966..b0b8ed4 100644 --- a/internal/action/service.go +++ b/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) diff --git a/pkg/qbittorrent/client.go b/pkg/qbittorrent/client.go index ee4b44c..575fbb0 100644 --- a/pkg/qbittorrent/client.go +++ b/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 { - c := &Client{ - settings: s, +func NewClient(settings Settings) Client { + c := &client{ + settings: settings, + Name: settings.Name, + log: log.New(io.Discard, "", log.LstdFlags), } - if s.Log == nil { - c.Log = log.New(io.Discard, "qbittorrent", log.LstdFlags) + // override logger if we pass one + 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) diff --git a/pkg/qbittorrent/methods.go b/pkg/qbittorrent/methods.go index 2893d4a..c37bdad 100644 --- a/pkg/qbittorrent/methods.go +++ b/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")