remove error return from ExtractPictures

That function returns an error in a never
expected condition, and that error would be
logged message on the caller side:
none of the callers handles it.

That change hides that error from the caller
so that function would have a signature that
better fit what it does and how it behaves.

backend/app/rest/api/rest_private.go

@@ -89,13 +89,8 @@ func (s *private) createCommentCtrl(w http.ResponseWriter, r *http.Request) {
 	comment = s.commentFormatter.Format(comment)
 
 	// check if images are valid
-	imgIds, err := s.imageService.ExtractPictures(comment.Text)
-	if err != nil {
-		rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't extract pictures from comment text", rest.ErrCommentValidation)
-		return
-	}
-	for _, id := range imgIds {
-		_, err = s.imageService.Load(id)
+	for _, id := range s.imageService.ExtractPictures(comment.Text) {
+		_, err := s.imageService.Load(id)
 		if err != nil {
 			rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't load picture from the comment", rest.ErrImgNotFound)
 			return

backend/app/rest/api/rest_public.go

@@ -134,12 +134,7 @@ func (s *public) previewCommentCtrl(w http.ResponseWriter, r *http.Request) {
 	comment.Sanitize()
 
 	// check if images are valid
-	imgIds, err := s.imageService.ExtractPictures(comment.Text)
-	if err != nil {
-		rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't extract pictures from comment text", rest.ErrCommentValidation)
-		return
-	}
-	for _, id := range imgIds {
+	for _, id := range s.imageService.ExtractPictures(comment.Text) {
 		_, err = s.imageService.Load(id)
 		if err != nil {
 			rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't load picture from the comment", rest.ErrImgNotFound)

backend/app/store/image/image.go

@@ -148,13 +148,12 @@ func (s *Service) Submit(idsFn func() []string) {
 }
 
 // ExtractPictures gets list of images from the doc html and convert from urls to ids, i.e. user/pic.png
-// It doesn't return possible errors parsing the cached images, and will try to return as many parsed ids
-// as possible instead.
-func (s *Service) ExtractPictures(commentHTML string) (ids []string, err error) {
+func (s *Service) ExtractPictures(commentHTML string) (ids []string) {
 
 	doc, err := goquery.NewDocumentFromReader(strings.NewReader(commentHTML))
 	if err != nil {
-		return nil, errors.Wrap(err, "can't create document")
+		log.Printf("[ERROR] can't parse commentHTML to parse images: %q, error: %v", commentHTML, err)
+		return nil
 	}
 	doc.Find("img").Each(func(i int, sl *goquery.Selection) {
 		if im, ok := sl.Attr("src"); ok {
@@ -183,7 +182,7 @@ func (s *Service) ExtractPictures(commentHTML string) (ids []string, err error)
 		}
 	})
 
-	return ids, nil
+	return ids
 }
 
 // Cleanup runs periodic cleanup with cleanupTTL. Blocking loop, should be called inside of goroutine by consumer

backend/app/store/image/image_test.go

@@ -77,8 +77,7 @@ func TestService_ExtractPictures(t *testing.T) {
 	svc := Service{ServiceParams: ServiceParams{ImageAPI: "/blah/", ProxyAPI: "/non_existent"}}
 	html := `blah <img src="/blah/user1/pic1.png"/> foo
 <img src="/blah/user2/pic3.png"/> xyz <p>123</p> <img src="/pic3.png"/> <img src="https://i.ibb.co/0cqqqnD/ezgif-5-3b07b6b97610.png" alt="">`
-	ids, err := svc.ExtractPictures(html)
-	require.NoError(t, err)
+	ids := svc.ExtractPictures(html)
 	require.Equal(t, 2, len(ids), "two images")
 	assert.Equal(t, "user1/pic1.png", ids[0])
 	assert.Equal(t, "user2/pic3.png", ids[1])
@@ -90,35 +89,30 @@ func TestService_ExtractPictures(t *testing.T) {
 <p><img src="https://remark42.radio-t.com/api/v1/picture/github_ef0f706a79cc24b17bbbb374cd234a691d034128/bjttt8ahajfmrhsula10.png" alt="bjtr0-201906-08110846-i324c.png"/></p>\n\n<p>
 По форме все верно, это все packages, но по сути это все одна библиотека организованная таким образом. При ее импорте, например посредством go mod, она выглядит как один модуль, т.е.
 <code>github.com/go-pkgz/auth v0.5.2</code>.</p>\n`
-	ids, err = svc.ExtractPictures(html)
-	require.NoError(t, err)
+	ids = svc.ExtractPictures(html)
 	require.Equal(t, 1, len(ids), "one image in")
 	assert.Equal(t, "github_ef0f706a79cc24b17bbbb374cd234a691d034128/bjttt8ahajfmrhsula10.png", ids[0])
 
 	// proxied image
 	html = `<img src="https://remark42.radio-t.com/api/v1/img?src=aHR0cHM6Ly9ob21lcGFnZXMuY2FlLndpc2MuZWR1L35lY2U1MzMvaW1hZ2VzL2JvYXQucG5n" alt="cat.png">`
-	ids, err = svc.ExtractPictures(html)
-	require.NoError(t, err)
+	ids = svc.ExtractPictures(html)
 	require.Equal(t, 1, len(ids), "one image in")
 	assert.Equal(t, "cached_images/12318fbd4c55e9d177b8b5ae197bc89c5afd8e07-a41fcb00643f28d700504256ec81cbf2e1aac53e", ids[0])
 
 	// bad url
 	html = `<img src=" https://remark42.radio-t.com/api/v1/img">`
-	ids, err = svc.ExtractPictures(html)
-	require.NoError(t, err)
+	ids = svc.ExtractPictures(html)
 	require.Empty(t, ids)
 
 	// bad src
 	html = `<img src="https://remark42.radio-t.com/api/v1/img?src=bad">`
-	ids, err = svc.ExtractPictures(html)
-	require.NoError(t, err)
+	ids = svc.ExtractPictures(html)
 	require.Empty(t, ids)
 
 	// good src with bad content
 	badURL := base64.URLEncoding.EncodeToString([]byte(" http://foo.bar"))
 	html = fmt.Sprintf(`<img src="https://remark42.radio-t.com/api/v1/img?src=%s">`, badURL)
-	ids, err = svc.ExtractPictures(html)
-	require.NoError(t, err)
+	ids = svc.ExtractPictures(html)
 	require.Empty(t, ids)
 }
 

backend/app/store/service/service.go

@@ -235,11 +235,7 @@ func (s *DataStore) submitImages(comment store.Comment) {
 			log.Printf("[WARN] can't get comment's %s text for image extraction, %v", comment.ID, err)
 			return nil
 		}
-		imgIds, err := s.ImageService.ExtractPictures(cc.Text)
-		if err != nil {
-			log.Printf("[WARN] can't get extract pictures from %s, %v", comment.ID, err)
-			return nil
-		}
+		imgIds := s.ImageService.ExtractPictures(cc.Text)
 		if len(imgIds) > 0 {
 			log.Printf("[DEBUG] image ids extracted from %s - %+v", comment.ID, imgIds)
 		}