Update changelog

CHANGELOG.md

@@ -3,6 +3,18 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
 
+## [v0.11.2] - 2021-10-19
+
+- Really fix the bug where too large attachments keep retrying
+- Fix tests on Windows
+
+## [v0.11.1] - 2021-10-11
+
+- Improve logging and error handling
+- Improve tests
+- Fix problem where attachments too large for discord fail immediately and do not retry
+- Fix problem with version checking
+
 ## [v0.11.0] - 2021-10-10
 
 - Switched to semantic versioning
@@ -59,4 +71,4 @@ Add --exclude option to avoid uploading files in thumbnail directories
 
 ## [0.1.0] - 2017-02-16
 
-Initial release
+Initial release

config/config_test.go

@@ -10,10 +10,14 @@ func TestNoConfig(t *testing.T) {
 	c := ConfigService{}
 
 	c.ConfigFilename = emptyTempFile()
-	os.Remove(c.ConfigFilename)
+	err := os.Remove(c.ConfigFilename)
+	if err != nil {
+		t.Fatalf("could not remove file: %v", err)
+	}
+
 	defer os.Remove(c.ConfigFilename) // because we are about to create it
 
-	err := c.LoadOrInit()
+	err = c.LoadOrInit()
 	if err != nil {
 		t.Errorf("unexpected failure from load: %s", err)
 	}
@@ -84,6 +88,7 @@ func emptyTempFile() string {
 	if err != nil {
 		panic(err)
 	}
+	f.Close()
 	return f.Name()
 }
 

dau_test.go

@@ -80,7 +80,11 @@ func TestCheckPath(t *testing.T) {
 	if !w.checkPath() {
 		t.Error("checkPath failed?")
 	}
-	os.RemoveAll(dir)
+
+	err := os.RemoveAll(dir)
+	if err != nil {
+		t.Fatalf("could not remove test dir: %v", err)
+	}
 	if w.checkPath() {
 		t.Error("checkPath succeeded when shouldn't?")
 	}
@@ -91,9 +95,11 @@ func createFileTree() string {
 	if err != nil {
 		log.Fatal(err)
 	}
-	os.Create(fmt.Sprintf("%s%c%s", dir, os.PathSeparator, "a.gif"))
+	f1, _ := os.Create(fmt.Sprintf("%s%c%s", dir, os.PathSeparator, "a.gif"))
-	os.Create(fmt.Sprintf("%s%c%s", dir, os.PathSeparator, "a.jpg"))
+	f2, _ := os.Create(fmt.Sprintf("%s%c%s", dir, os.PathSeparator, "a.jpg"))
-	os.Create(fmt.Sprintf("%s%c%s", dir, os.PathSeparator, "a.png"))
+	f3, _ := os.Create(fmt.Sprintf("%s%c%s", dir, os.PathSeparator, "a.png"))
-
+	f1.Close()
+	f2.Close()
+	f3.Close()
 	return dir
 }

go.mod

@@ -6,7 +6,6 @@ require (
 	github.com/fogleman/gg v1.3.0
 	github.com/golang/freetype v0.0.0-20170609003504-e2365dfdc4a0 // indirect
 	github.com/mitchellh/go-homedir v1.1.0
-	github.com/pborman/getopt v1.1.0
 	golang.org/x/image v0.0.0-20210504121937-7319ad40d33e
 	golang.org/x/mod v0.4.2
 )

go.sum

@@ -4,8 +4,6 @@ github.com/golang/freetype v0.0.0-20170609003504-e2365dfdc4a0 h1:DACJavvAHhabrF0
 github.com/golang/freetype v0.0.0-20170609003504-e2365dfdc4a0/go.mod h1:E/TSTwGwJL78qG/PmXZO1EjYhfJinVAhrmmHX6Z8B9k=
 github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y=
 github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
-github.com/pborman/getopt v1.1.0 h1:eJ3aFZroQqq0bWmraivjQNt6Dmm5M0h2JcDW38/Azb0=
-github.com/pborman/getopt v1.1.0/go.mod h1:FxXoW1Re00sQG/+KIkuSqRL/LwQgSkv7uyac+STFsbk=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/image v0.0.0-20210504121937-7319ad40d33e h1:PzJMNfFQx+QO9hrC1GwZ4BoPGeNGhfeQEgcQFArEjPk=

log/log.go

@@ -37,11 +37,10 @@ func init() {
 }
 
 func SendLog(entry string, entryType LogEntryType) {
-
 	logInput <- LogEntry{
 		Timestamp: time.Now(),
 		Entry:     entry,
 		Type:      entryType,
 	}
-	log.Printf(entry)
+	log.Printf("%6s: %s", entryType, entry)
 }

upload/upload.go

@@ -5,6 +5,7 @@ package upload
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"image"
 	"io"
@@ -22,6 +23,10 @@ import (
 	"golang.org/x/image/font/inconsolata"
 )
 
+type HTTPClient interface {
+	Do(req *http.Request) (*http.Response, error)
+}
+
 type Uploader struct {
 	Uploads []*Upload `json:"uploads"`
 }
@@ -30,6 +35,8 @@ type Upload struct {
 	Uploaded   bool      `json:"uploaded"` // has this file been uploaded to discord
 	UploadedAt time.Time `json:"uploaded_at"`
 
+	Failed bool `json:"failed"`
+
 	originalFilename string // path on the local disk
 	filenameToUpload string // post-watermark, or just original if unwatermarked
 
@@ -43,6 +50,8 @@ type Upload struct {
 
 	Width  int `json:"width"`
 	Height int `json:"height"`
+
+	Client HTTPClient `json:"-"`
 }
 
 func NewUploader() *Uploader {
@@ -67,20 +76,22 @@ func (u *Uploader) AddFile(file string, conf config.Watcher) {
 func (u *Uploader) Upload() {
 
 	for _, upload := range u.Uploads {
-		if !upload.Uploaded {
+		if !upload.Uploaded && !upload.Failed {
 			upload.processUpload()
 		}
 	}
 }
 
-func (u *Upload) processUpload() {
+func (u *Upload) processUpload() error {
+	daulog.SendLog(fmt.Sprintf("Uploading: %s", u.originalFilename), daulog.LogTypeInfo)
+
 	if u.webhookURL == "" {
 		daulog.SendLog("WebHookURL is not configured - cannot upload!", daulog.LogTypeError)
-		return
+		return errors.New("webhook url not configured")
 	}
 
 	if u.watermark {
-		daulog.SendLog("Watermarking", daulog.LogTypeInfo)
+		daulog.SendLog("Watermarking image", daulog.LogTypeInfo)
 		u.applyWatermark()
 	} else {
 		u.filenameToUpload = u.originalFilename
@@ -112,17 +123,30 @@ func (u *Upload) processUpload() {
 
 		request, err := newfileUploadRequest(u.webhookURL, extraParams, "file", u.filenameToUpload)
 		if err != nil {
-			log.Fatal(err)
+			log.Printf("error creating upload request: %s", err)
+			return fmt.Errorf("could not create upload request: %s", err)
 		}
 		start := time.Now()
-		client := &http.Client{Timeout: time.Second * 30}
+
-		resp, err := client.Do(request)
+		if u.Client == nil {
+			// if no client was specified (a unit test) then create
+			// a default one
+			u.Client = &http.Client{Timeout: time.Second * 30}
+		}
+
+		resp, err := u.Client.Do(request)
 		if err != nil {
 			log.Print("Error performing request:", err)
 			retriesRemaining--
 			sleepForRetries(retriesRemaining)
 			continue
 		} else {
+			if resp.StatusCode == 413 {
+				// just fail immediately, we know this means the file was too big
+				daulog.SendLog("413 received - file too large", daulog.LogTypeError)
+				u.Failed = true
+				return errors.New("received 413 - file too large")
+			}
 
 			if resp.StatusCode != 200 {
 				// {"message": "Request entity too large", "code": 40005}
@@ -189,13 +213,16 @@ func (u *Upload) processUpload() {
 
 	if retriesRemaining == 0 {
 		daulog.SendLog("Failed to upload, even after all retries", daulog.LogTypeError)
+		u.Failed = true
+		return errors.New("could not upload after all retries")
 	}
+	return nil
 }
 
 func newfileUploadRequest(uri string, params map[string]string, paramName, path string) (*http.Request, error) {
 	file, err := os.Open(path)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("could not open file '%s': %s", path, err)
 	}
 	defer file.Close()
 

upload/upload_test.go

@@ -0,0 +1,72 @@
+package upload
+
+import (
+	"bytes"
+	"io/ioutil"
+	"net/http"
+	"os"
+	"testing"
+	// 	"github.com/tardisx/discord-auto-upload/config"
+)
+
+// https://www.thegreatcodeadventure.com/mocking-http-requests-in-golang/
+type MockClient struct {
+	DoFunc func(req *http.Request) (*http.Response, error)
+}
+
+func (m *MockClient) Do(req *http.Request) (*http.Response, error) {
+	return m.DoFunc(req)
+}
+
+func DoGoodUpload(req *http.Request) (*http.Response, error) {
+	r := ioutil.NopCloser(bytes.NewReader([]byte(`{"id": "123456789012345678", "type": 0, "content": "", "channel_id": "849615269706203171", "author": {"bot": true, "id": "849615314274484224", "username": "abcdedf", "avatar": null, "discriminator": "0000"}, "attachments": [{"id": "851092588332449812", "filename": "dau480457962.png", "size": 859505, "url": "https://cdn.discordapp.com/attachments/849615269706203171/851092588332449812/dau480457962.png", "proxy_url": "https://media.discordapp.net/attachments/849615269706203171/851092588332449812/dau480457962.png", "width": 640, "height": 640, "content_type": "image/png"}], "embeds": [], "mentions": [], "mention_roles": [], "pinned": false, "mention_everyone": false, "tts": false, "timestamp": "2021-06-06T13:38:05.660000+00:00", "edited_timestamp": null, "flags": 0, "components": [], "webhook_id": "123456789012345678"}`)))
+	return &http.Response{
+		StatusCode: 200,
+		Body:       r,
+	}, nil
+}
+
+func DoTooBigUpload(req *http.Request) (*http.Response, error) {
+	r := ioutil.NopCloser(bytes.NewReader([]byte(`{"message": "Request entity too large", "code": 40005}`)))
+	return &http.Response{
+		StatusCode: 413,
+		Body:       r,
+	}, nil
+}
+
+func TestSuccessfulUpload(t *testing.T) {
+	// create temporary file, processUpload requires that it exists, even though
+	// we will not really be uploading it here
+	f, _ := os.CreateTemp("", "dautest-upload-*")
+	defer os.Remove(f.Name())
+	u := Upload{webhookURL: "https://127.0.0.1/", originalFilename: f.Name()}
+	u.Client = &MockClient{DoFunc: DoGoodUpload}
+	err := u.processUpload()
+	if err != nil {
+		t.Errorf("error occured: %s", err)
+	}
+	if u.Width != 640 || u.Height != 640 {
+		t.Error("dimensions wrong")
+	}
+	if u.Url != "https://cdn.discordapp.com/attachments/849615269706203171/851092588332449812/dau480457962.png" {
+		t.Error("URL wrong")
+	}
+}
+
+func TestTooBigUpload(t *testing.T) {
+	// create temporary file, processUpload requires that it exists, even though
+	// we will not really be uploading it here
+	f, _ := os.CreateTemp("", "dautest-upload-*")
+	defer os.Remove(f.Name())
+	u := Upload{webhookURL: "https://127.0.0.1/", originalFilename: f.Name()}
+	u.Client = &MockClient{DoFunc: DoTooBigUpload}
+	err := u.processUpload()
+	if err == nil {
+		t.Error("error did not occur?")
+	} else if err.Error() != "received 413 - file too large" {
+		t.Errorf("wrong error occurred: %s", err.Error())
+	}
+	if u.Failed != true {
+		t.Error("upload should have been marked failed")
+	}
+}

version/version.go

@@ -7,7 +7,7 @@ import (
 	"golang.org/x/mod/semver"
 )
 
-const CurrentVersion string = "v0.11.0"
+const CurrentVersion string = "v0.11.2"
 
 func NewVersionAvailable(v string) bool {
 	if !semver.IsValid(CurrentVersion) {
@@ -22,7 +22,7 @@ func NewVersionAvailable(v string) bool {
 	if comp == 0 {
 		return false
 	}
-	if comp == -1 {
+	if comp == 1 {
 		return true
 	}
 	return false // they are using a newer one than exists?

version/version_test.go

@@ -5,7 +5,7 @@ import (
 )
 
 func TestVersioning(t *testing.T) {
-	if !NewVersionAvailable("v0.1.0") {
+	if !NewVersionAvailable("v1.0.0") {
-		t.Error("should be a version newer than v0.1.0")
+		t.Error("should be a version newer than v1.0.0")
 	}
 }

web/server.go

@@ -152,7 +152,13 @@ func (ws *WebService) handleConfig(w http.ResponseWriter, r *http.Request) {
 func (ws *WebService) getUploads(w http.ResponseWriter, r *http.Request) {
 	w.Header().Set("Content-Type", "application/json")
 	ups := ws.Uploader.Uploads
-	text, _ := json.Marshal(ups)
+
+	text, err := json.Marshal(ups)
+	if err != nil {
+		daulog.SendLog(fmt.Sprintf("err: %v", err), daulog.LogTypeError)
+		w.Write([]byte("could not marshall uploads?"))
+		return
+	}
 	w.Write([]byte(text))
 }