Skip to content

Commit

Permalink
chore(feeds): better error handling for invalid feed files
Browse files Browse the repository at this point in the history
When a user uploads an invalid feed file we do not have any mechanism
currently to notify that user.  Further will continue to run the file
through our rss-to-email cron.

This commit makes it so we perform validation on the feed files as they
are uploaded and refuse to save files that we know will eventually
fail.

Further, we don't want to continuously try files that we know will not
succeed so we are pushing those known issues into our retry-then-delete
mechanism inside our cron.
neurosnap committed Jan 10, 2025
1 parent 3eda1b9 commit 4a540a6
Showing 2 changed files with 53 additions and 23 deletions.
29 changes: 20 additions & 9 deletions feeds/cron.go
Original file line number Diff line number Diff line change
@@ -9,6 +9,7 @@ import (
"log/slog"
"math"
"net/http"
"net/url"
"strings"
"text/template"
"time"
@@ -161,23 +162,29 @@ func (f *Fetcher) RunPost(logger *slog.Logger, user *db.User, post *db.Post) err

urls := []string{}
for _, item := range parsed.Items {
url := ""
if item.IsText {
url = item.Value
u := ""
if item.IsText || item.IsURL {
u = item.Value
} else if item.IsURL {
url = string(item.URL)
u = string(item.Value)
}

if url == "" {
if u == "" {
continue
}

urls = append(urls, url)
_, err := url.Parse(string(item.URL))
if err != nil {
logger.Info("invalid url", "url", string(item.URL))
continue
}

urls = append(urls, u)
}

now := time.Now().UTC()
if post.ExpiresAt == nil {
expiresAt := time.Now().AddDate(0, 6, 0)
expiresAt := time.Now().AddDate(0, 12, 0)
post.ExpiresAt = &expiresAt
}
_, err = f.db.UpdatePost(post)
@@ -199,7 +206,7 @@ func (f *Fetcher) RunPost(logger *slog.Logger, user *db.User, post *db.Post) err
post.Data.Attempts += 1
logger.Error("could not fetch urls", "err", err, "attempts", post.Data.Attempts)

errBody := fmt.Sprintf(`There was an error attempting to fetch your feeds (%d) times. After (3) attempts we remove the file from our system. Please check all the URLs and re-upload.
errBody := fmt.Sprintf(`There was an error attempting to fetch your feeds (%d) times. After (5) attempts we remove the file from our system. Please check all the URLs and re-upload.
Also, we have centralized logs in our pico.sh TUI that will display realtime feed errors so you can debug.
@@ -217,7 +224,7 @@ Also, we have centralized logs in our pico.sh TUI that will display realtime fee
return err
}

if post.Data.Attempts >= 3 {
if post.Data.Attempts >= 5 {
err = f.db.RemovePosts([]string{post.ID})
if err != nil {
return err
@@ -410,6 +417,10 @@ func (f *Fetcher) FetchAll(logger *slog.Logger, urls []string, inlineContent boo
return nil, err
}

if len(urls) == 0 {
return nil, fmt.Errorf("feed file does not contain any urls")
}

var allErrors error
for _, url := range urls {
feedTmpl, err := f.Fetch(logger, fp, url, username, feedItems)
47 changes: 33 additions & 14 deletions feeds/scp_hooks.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
package feeds

import (
"errors"
"fmt"
"net/url"

"strings"
"time"

"slices"

"github.com/charmbracelet/ssh"
"github.com/picosh/pico/db"
"github.com/picosh/pico/filehandlers"
@@ -38,23 +39,41 @@ func (p *FeedHooks) FileValidate(s ssh.Session, data *filehandlers.PostMetaData)
return false, err
}

return true, nil
}

func (p *FeedHooks) FileMeta(s ssh.Session, data *filehandlers.PostMetaData) error {
parsedText := shared.ListParseText(string(data.Text))
// Because we need to support sshfs, sftp runs our Write handler twice
// and on the first pass we do not have access to the file data.
// In that case we should skip the parsing validation
if data.Text == "" {
return true, nil
}

if parsedText.Title == "" {
data.Title = utils.ToUpper(data.Slug)
} else {
data.Title = parsedText.Title
parsed := shared.ListParseText(string(data.Text))
if parsed.Email == "" {
return false, fmt.Errorf("ERROR: no email variable detected for %s, check the format of your file, skipping", data.Filename)
}

data.Description = parsedText.Description
data.Tags = parsedText.Tags
var allErr error
for _, txt := range parsed.Items {
u := ""
if txt.IsText {
u = txt.Value
} else if txt.IsURL {
u = string(txt.URL)
}

data.Hidden = slices.Contains(p.Cfg.HiddenPosts, data.Filename)
_, err := url.Parse(u)
if err != nil {
allErr = errors.Join(allErr, fmt.Errorf("%s: %w", u, err))
continue
}
}
if allErr != nil {
return false, fmt.Errorf("ERROR: some urls provided were invalid check the format of your file, skipping: %w", allErr)
}

return true, nil
}

func (p *FeedHooks) FileMeta(s ssh.Session, data *filehandlers.PostMetaData) error {
if data.Data.LastDigest == nil {
now := time.Now()
data.Data.LastDigest = &now

0 comments on commit 4a540a6

Please sign in to comment.