pico
created pr with
41.1
added 41.2
1: 5d5b1f5 ! 1: a6f10fc chore(feeds): better error handling for invalid feed files
cmds
checkout latest patchset:
ssh pr.pico.sh print 41 | git am -3checkout any patchset in a patch request:
ssh pr.pico.sh print 41.[rev] | git am -3add changes to patch request:
git format-patch main --stdout | ssh pr.pico.sh pr add 41
Patchset
41.2
chore(feeds): better error handling for invalid feed files
Eric Bower
2025-01-10T02:48:45ZWhen a user uploads an invalid feed file we do not have any mechanism to notify that user. Further will continue to attempt 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 refused 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.
Semantic diff summary
0 added,
6 modified,
0 signature changed,
0 removed
across 2 analyzed files
+20
-9
feeds/cron.go
#
| ... | ... | @@ -161,23 +162,29 @@ func (f *Fetcher) RunPost(logger *slog.Logger, user *db.User, post *db.Post) err | |
| 161 | 162 | ||
| 162 | 163 | urls := []string{} | |
| 163 | 164 | for _, item := range parsed.Items { | |
| 164 | - | url := "" | |
| 165 | - | if item.IsText { | |
| 166 | - | url = item.Value | |
| 165 | + | u := "" | |
| 166 | + | if item.IsText || item.IsURL { | |
| 167 | + | u = item.Value | |
| 167 | 168 | } else if item.IsURL { | |
| 168 | - | url = string(item.URL) | |
| 169 | + | u = string(item.Value) | |
| 169 | 170 | } | |
| 170 | 171 | ||
| 171 | - | if url == "" { | |
| 172 | + | if u == "" { | |
| 172 | 173 | continue | |
| 173 | 174 | } | |
| 174 | 175 | ||
| 175 | - | urls = append(urls, url) | |
| 176 | + | _, err := url.Parse(string(item.URL)) | |
| 177 | + | if err != nil { | |
| 178 | + | logger.Info("invalid url", "url", string(item.URL)) | |
| 179 | + | continue | |
| 180 | + | } | |
| 181 | + | ||
| 182 | + | urls = append(urls, u) | |
| 176 | 183 | } | |
| 177 | 184 | ||
| 178 | 185 | now := time.Now().UTC() | |
| 179 | 186 | if post.ExpiresAt == nil { | |
| 180 | - | expiresAt := time.Now().AddDate(0, 6, 0) | |
| 187 | + | expiresAt := time.Now().AddDate(0, 12, 0) | |
| 181 | 188 | post.ExpiresAt = &expiresAt | |
| 182 | 189 | } | |
| 183 | 190 | _, err = f.db.UpdatePost(post) |
| ... | ... | @@ -199,7 +206,7 @@ func (f *Fetcher) RunPost(logger *slog.Logger, user *db.User, post *db.Post) err | |
| 199 | 206 | post.Data.Attempts += 1 | |
| 200 | 207 | logger.Error("could not fetch urls", "err", err, "attempts", post.Data.Attempts) | |
| 201 | 208 | ||
| 202 | - | 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. | |
| 209 | + | 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. | |
| 203 | 210 | Also, we have centralized logs in our pico.sh TUI that will display realtime feed errors so you can debug. | |
| 204 | 211 | ||
| 205 | 212 |
| ... | ... | @@ -410,6 +417,10 @@ func (f *Fetcher) FetchAll(logger *slog.Logger, urls []string, inlineContent boo | |
| 410 | 417 | return nil, err | |
| 411 | 418 | } | |
| 412 | 419 | ||
| 420 | + | if len(urls) == 0 { | |
| 421 | + | return nil, fmt.Errorf("feed file does not contain any urls") | |
| 422 | + | } | |
| 423 | + | ||
| 413 | 424 | var allErrors error | |
| 414 | 425 | for _, url := range urls { | |
| 415 | 426 | feedTmpl, err := f.Fetch(logger, fp, url, username, feedItems) |
+33
-14
feeds/scp_hooks.go
#
| ... | ... | @@ -38,23 +39,41 @@ func (p *FeedHooks) FileValidate(s ssh.Session, data *filehandlers.PostMetaData) | |
| 38 | 39 | return false, err | |
| 39 | 40 | } | |
| 40 | 41 | ||
| 41 | - | return true, nil | |
| 42 | - | } | |
| 43 | - | ||
| 44 | - | func (p *FeedHooks) FileMeta(s ssh.Session, data *filehandlers.PostMetaData) error { | |
| 45 | - | parsedText := shared.ListParseText(string(data.Text)) | |
| 42 | + | // Because we need to support sshfs, sftp runs our Write handler twice | |
| 43 | + | // and on the first pass we do not have access to the file data. | |
| 44 | + | // In that case we should skip the parsing validation | |
| 45 | + | if data.Text == "" { | |
| 46 | + | return true, nil | |
| 47 | + | } | |
| 46 | 48 | ||
| 47 | - | if parsedText.Title == "" { | |
| 48 | - | data.Title = utils.ToUpper(data.Slug) | |
| 49 | - | } else { | |
| 50 | - | data.Title = parsedText.Title | |
| 49 | + | parsed := shared.ListParseText(string(data.Text)) | |
| 50 | + | if parsed.Email == "" { | |
| 51 | + | return false, fmt.Errorf("ERROR: no email variable detected for %s, check the format of your file, skipping", data.Filename) | |
| 51 | 52 | } | |
| 52 | 53 | ||
| 53 | - | data.Description = parsedText.Description | |
| 54 | - | data.Tags = parsedText.Tags | |
| 54 | + | var allErr error | |
| 55 | + | for _, txt := range parsed.Items { | |
| 56 | + | u := "" | |
| 57 | + | if txt.IsText { | |
| 58 | + | u = txt.Value | |
| 59 | + | } else if txt.IsURL { | |
| 60 | + | u = string(txt.URL) | |
| 61 | + | } | |
| 55 | 62 | ||
| 56 | - | data.Hidden = slices.Contains(p.Cfg.HiddenPosts, data.Filename) | |
| 63 | + | _, err := url.Parse(u) | |
| 64 | + | if err != nil { | |
| 65 | + | allErr = errors.Join(allErr, fmt.Errorf("%s: %w", u, err)) | |
| 66 | + | continue | |
| 67 | + | } | |
| 68 | + | } | |
| 69 | + | if allErr != nil { | |
| 70 | + | return false, fmt.Errorf("ERROR: some urls provided were invalid check the format of your file, skipping: %w", allErr) | |
| 71 | + | } | |
| 72 | + | ||
| 73 | + | return true, nil | |
| 74 | + | } | |
| 57 | 75 | ||
| 76 | + | func (p *FeedHooks) FileMeta(s ssh.Session, data *filehandlers.PostMetaData) error { | |
| 58 | 77 | if data.Data.LastDigest == nil { | |
| 59 | 78 | now := time.Now() | |
| 60 | 79 | data.Data.LastDigest = &now |