Do not panic

vanng822 · Apr 5, 2019 · da22702 · da22702
1 parent f0a5600
commit da22702
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 28 deletions.
diff --git a/premailer/premailer_from_file.go b/premailer/premailer_from_file.go
@@ -1,24 +1,24 @@
 package premailer
 
 import (
-	"github.com/PuerkitoBio/goquery"
 	"os"
+
+	"github.com/PuerkitoBio/goquery"
 )
 
 // NewPremailerFromFile take an filename
 // Read the content of this file
 // and create a goquery.Document
 // and then create and Premailer instance.
-// It will panic if any error happens
-func NewPremailerFromFile(filename string, options *Options) Premailer {
+func NewPremailerFromFile(filename string, options *Options) (Premailer, error) {
 	fd, err := os.Open(filename)
 	if err != nil {
-		panic(err)
+		return nil, err
 	}
 	defer fd.Close()
 	d, err := goquery.NewDocumentFromReader(fd)
 	if err != nil {
-		panic(err)
+		return nil, err
 	}
-	return NewPremailer(d, options)
+	return NewPremailer(d, options), nil
 }
diff --git a/premailer/premailer_from_file_test.go b/premailer/premailer_from_file_test.go
@@ -1,12 +1,14 @@
 package premailer
 
 import (
-	"github.com/stretchr/testify/assert"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestBasicHTMLFromFile(t *testing.T) {
-	p := NewPremailerFromFile("data/markup_test.html", nil)
+	p, err := NewPremailerFromFile("data/markup_test.html", nil)
+	assert.Nil(t, err)
 	result_html, err := p.Transform()
 	assert.Nil(t, err)
 
@@ -17,8 +19,8 @@ func TestBasicHTMLFromFile(t *testing.T) {
 	assert.Contains(t, result_html, "<div style=\"background-color:green\" bgcolor=\"green\">Green color</div>")
 }
 
-func TestFromFilePanic(t *testing.T) {
-	assert.Panics(t, func() {
-		NewPremailerFromFile("data/blablabla.html", nil)
-	})
+func TestFromFileNotFound(t *testing.T) {
+	p, err := NewPremailerFromFile("data/blablabla.html", nil)
+	assert.NotNil(t, err)
+	assert.Nil(t, p)
 }
diff --git a/premailer/premailer_from_string.go b/premailer/premailer_from_string.go
@@ -1,19 +1,19 @@
 package premailer
 
 import (
-	"github.com/PuerkitoBio/goquery"
 	"strings"
+
+	"github.com/PuerkitoBio/goquery"
 )
 
 // NewPremailerFromString take in a document in string format
 // and create a goquery.Document
 // and then create and Premailer instance.
-// It will panic if any error happens
-func NewPremailerFromString(doc string, options *Options) Premailer {
+func NewPremailerFromString(doc string, options *Options) (Premailer, error) {
 	read := strings.NewReader(doc)
 	d, err := goquery.NewDocumentFromReader(read)
 	if err != nil {
-		panic(err)
+		return nil, err
 	}
-	return NewPremailer(d, options)
+	return NewPremailer(d, options), nil
 }
diff --git a/premailer/premailer_test.go b/premailer/premailer_test.go
@@ -38,7 +38,8 @@ func TestBasicHTML(t *testing.T) {
         </body>
         </html>`
 
-	p := NewPremailerFromString(html, nil)
+	p, err := NewPremailerFromString(html, nil)
+	assert.Nil(t, err)
 	result_html, err := p.Transform()
 	assert.Nil(t, err)
 
@@ -68,7 +69,8 @@ func TestDataPremailerIgnore(t *testing.T) {
         </body>
         </html>`
 
-	p := NewPremailerFromString(html, nil)
+	p, err := NewPremailerFromString(html, nil)
+	assert.Nil(t, err)
 	result_html, err := p.Transform()
 	assert.Nil(t, err)
 
@@ -96,7 +98,8 @@ func TestWithInline(t *testing.T) {
         </body>
         </html>`
 
-	p := NewPremailerFromString(html, nil)
+	p, err := NewPremailerFromString(html, nil)
+	assert.Nil(t, err)
 	result_html, err := p.Transform()
 	assert.Nil(t, err)
 
@@ -128,7 +131,8 @@ func TestPseudoSelectors(t *testing.T) {
         </body>
         </html>`
 
-	p := NewPremailerFromString(html, nil)
+	p, err := NewPremailerFromString(html, nil)
+	assert.Nil(t, err)
 	result_html, err := p.Transform()
 	assert.Nil(t, err)
 
@@ -158,7 +162,8 @@ func TestRemoveClass(t *testing.T) {
 
 	options := &Options{}
 	options.RemoveClasses = true
-	p := NewPremailerFromString(html, options)
+	p, err := NewPremailerFromString(html, options)
+	assert.Nil(t, err)
 	result_html, err := p.Transform()
 	assert.Nil(t, err)
 
@@ -187,7 +192,8 @@ func TestCssToAttributesFalse(t *testing.T) {
 
 	options := &Options{}
 	options.CssToAttributes = false
-	p := NewPremailerFromString(html, options)
+	p, err := NewPremailerFromString(html, options)
+	assert.Nil(t, err)
 	result_html, err := p.Transform()
 	assert.Nil(t, err)
 
@@ -218,7 +224,8 @@ func TestWithImportant(t *testing.T) {
         </body>
         </html>`
 
-	p := NewPremailerFromString(html, NewOptions())
+	p, err := NewPremailerFromString(html, NewOptions())
+	assert.Nil(t, err)
 	result_html, err := p.Transform()
 	assert.Nil(t, err)
 
@@ -257,7 +264,8 @@ func TestWithMediaRule(t *testing.T) {
         </body>
         </html>`
 
-	p := NewPremailerFromString(html, NewOptions())
+	p, err := NewPremailerFromString(html, NewOptions())
+	assert.Nil(t, err)
 	result_html, err := p.Transform()
 	assert.Nil(t, err)
 
@@ -311,7 +319,8 @@ func TestWithMediaAttribute(t *testing.T) {
         </body>
         </html>`
 
-	p := NewPremailerFromString(html, NewOptions())
+	p, err := NewPremailerFromString(html, NewOptions())
+	assert.Nil(t, err)
 	result_html, err := p.Transform()
 	assert.Nil(t, err)
 
@@ -365,7 +374,8 @@ func TestIndexOutOfRange(t *testing.T) {
         </body>
         </html>`
 
-	p := NewPremailerFromString(html, NewOptions())
+	p, err := NewPremailerFromString(html, NewOptions())
+	assert.Nil(t, err)
 	result_html, err := p.Transform()
 	assert.Nil(t, err)
 
@@ -407,7 +417,8 @@ func TestSpecificity(t *testing.T) {
         </body>
         </html>`
 
-	p := NewPremailerFromString(html, NewOptions())
+	p, err := NewPremailerFromString(html, NewOptions())
+	assert.Nil(t, err)
 	result_html, err := p.Transform()
 	assert.Nil(t, err)