Skip to content

Commit b8ca07f

Browse files
committed
Handle file errors, fix logAndPrint
Stop using fileExists. It not only introduces a race condition before os.Open(), but it also ignored some errors. Instaed use os.OpenFile and take not on the error returned make logAndPrint log from it's caller...
1 parent 90ba982 commit b8ca07f

File tree

2 files changed

+44
-36
lines changed

2 files changed

+44
-36
lines changed

grabbit.go

Lines changed: 38 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -41,31 +41,21 @@ type config struct {
4141
Subreddits []subreddit
4242
}
4343

44-
// fileExists checks if a file exists and is not a directory before we
45-
// try using it to prevent further errors. Notably, it ignores other errors LOL
46-
func fileExists(fileName string) bool {
47-
// https://golangcode.com/check-if-a-file-exists/
48-
info, err := os.Stat(fileName)
49-
if os.IsNotExist(err) {
50-
return false
51-
}
52-
return !info.IsDir()
53-
}
54-
44+
// downloadFile does not overwrite existing files
5545
func downloadFile(URL string, fileName string) error {
56-
// https://golangbyexample.com/download-image-file-url-golang/
5746

58-
response, err := http.Get(URL)
47+
// O_EXCL - used with O_CREATE, file must not exist
48+
file, err := os.OpenFile(fileName, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0666)
5949
if err != nil {
6050
return errors.WithStack(err)
6151
}
62-
defer response.Body.Close()
52+
defer file.Close()
6353

64-
file, err := os.Create(fileName)
54+
response, err := http.Get(URL)
6555
if err != nil {
6656
return errors.WithStack(err)
6757
}
68-
defer file.Close()
58+
defer response.Body.Close()
6959

7060
_, err = io.Copy(file, response.Body)
7161
if err != nil {
@@ -75,7 +65,6 @@ func downloadFile(URL string, fileName string) error {
7565
}
7666

7767
func genFilePath(destinationDir string, subreddit string, title string, fullURL string) (string, error) {
78-
// https://www.golangprograms.com/golang-download-image-from-given-url.html
7968
fileURL, err := url.Parse(fullURL)
8069
if err != nil {
8170
return "", errors.WithStack(err)
@@ -84,15 +73,28 @@ func genFilePath(destinationDir string, subreddit string, title string, fullURL
8473
path := fileURL.Path
8574
segments := strings.Split(path, "/")
8675

87-
fileName := segments[len(segments)-1]
76+
urlFileName := segments[len(segments)-1]
8877

8978
for _, s := range []string{" ", "/", "\\", "\n", "\r", "\x00"} {
9079
title = strings.ReplaceAll(title, s, "_")
9180
subreddit = strings.ReplaceAll(subreddit, s, "_")
9281
}
93-
fileName = subreddit + "_" + title + "_" + fileName
94-
fileName = filepath.Join(destinationDir, fileName)
95-
return fileName, nil
82+
fullFileName := subreddit + "_" + title + "_" + urlFileName
83+
filePath := filepath.Join(destinationDir, fullFileName)
84+
85+
// remove chars from title if it's too long for the OS to handle
86+
const MAX_PATH = 250
87+
if len(filePath) > MAX_PATH {
88+
toChop := len(filePath) - MAX_PATH
89+
if toChop > len(title) {
90+
return "", errors.Errorf("filePath to long and title too short: %#v\n", filePath)
91+
}
92+
93+
title = title[:len(title)-toChop]
94+
fullFileName = subreddit + "_" + title + "_" + urlFileName
95+
filePath = filepath.Join(destinationDir, fullFileName)
96+
}
97+
return filePath, nil
9698
}
9799

98100
func parseConfig(configBytes []byte) (*lumberjack.Logger, []subreddit, error) {
@@ -203,21 +205,23 @@ func grab(sugar *zap.SugaredLogger, subreddits []subreddit) error {
203205
)
204206
continue
205207
}
206-
if fileExists(filePath) {
207-
logAndPrint(sugar, os.Stdout,
208-
"file already exists!",
209-
"filePath", filePath,
210-
)
211-
continue
212-
}
213208
err = downloadFile(post.URL, filePath)
214209
if err != nil {
215-
logAndPrint(sugar, os.Stderr,
216-
"downloadFile error",
217-
"subreddit", subreddit.Name,
218-
"url", post.URL,
219-
"err", errors.WithStack(err),
220-
)
210+
if os.IsExist(errors.Cause(err)) {
211+
logAndPrint(sugar, os.Stdout,
212+
"file exists!",
213+
"subreddit", subreddit.Name,
214+
"filePath", filePath,
215+
"url", post.URL,
216+
)
217+
} else {
218+
logAndPrint(sugar, os.Stderr,
219+
"downloadFile error",
220+
"subreddit", subreddit.Name,
221+
"url", post.URL,
222+
"err", errors.WithStack(err),
223+
)
224+
}
221225
continue
222226

223227
}

logutil.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,16 @@ import (
1414
// fp == os.Stdout, Error if fp == os.Stderr, and panics otherwise.
1515
func logAndPrint(sugar *zap.SugaredLogger, fp *os.File, msg string, keysAndValues ...interface{}) {
1616

17+
// adjust stack frame to see caller of logAndPrint in the log
18+
// NOTE: could cache this somehow - make it an arg?
19+
callerSugar := sugar.Desugar().WithOptions(zap.AddCallerSkip(1)).Sugar()
20+
1721
switch fp {
1822
case os.Stdout:
19-
sugar.Infow(msg, keysAndValues...)
23+
callerSugar.Infow(msg, keysAndValues...)
2024
msg = "INFO: " + msg
2125
case os.Stderr:
22-
sugar.Errorw(msg, keysAndValues...)
26+
callerSugar.Errorw(msg, keysAndValues...)
2327
msg = "ERROR: " + msg
2428
default:
2529
sugar.Panicw(

0 commit comments

Comments
 (0)