Returns an Internal Server Error on grpc/db timeouts (letsencrypt#2624)

lucasamorimca · jsha · commit 3c4873bb5c87 · 2017-04-17T19:50:03.000-07:00
diff --git a/mocks/mocks.go b/mocks/mocks.go
@@ -1,6 +1,7 @@
 package mocks
 
 import (
+	"database/sql"
 	"encoding/json"
 	"encoding/pem"
 	"errors"
@@ -202,9 +203,11 @@ func (sa *StorageAuthority) GetAuthorization(_ context.Context, id string) (core
 		authz.Expires = &exp
 		authz.Challenges[0].URI = "http://localhost:4300/acme/challenge/expired/23"
 		return authz, nil
+	} else if id == "error_result" {
+		return core.Authorization{}, fmt.Errorf("Unspecified database error")
 	}
 
-	return core.Authorization{}, fmt.Errorf("authz not found")
+	return core.Authorization{}, sql.ErrNoRows
 }
 
 // RevokeAuthorizationsByDomain is a mock
diff --git a/sa/sa.go b/sa/sa.go
@@ -172,13 +172,7 @@ func (ssa *SQLStorageAuthority) GetAuthorization(ctx context.Context, id string)
 	if err == sql.ErrNoRows {
 		var fa authzModel
 		err = tx.SelectOne(&fa, fmt.Sprintf("SELECT %s FROM authz WHERE id = ?", authzFields), id)
-		if err == sql.ErrNoRows {
-			err = fmt.Errorf("No pendingAuthorization or authz with ID %s", id)
-			err = Rollback(tx, err)
-			return
-		}
 		if err != nil {
-			err = Rollback(tx, err)
 			return
 		}
 		authz = fa.Authorization
diff --git a/wfe/wfe.go b/wfe/wfe.go
@@ -3,6 +3,7 @@ package wfe
 import (
 	"bytes"
 	"crypto/x509"
+	"database/sql"
 	"encoding/hex"
 	"encoding/json"
 	"errors"
@@ -986,8 +987,11 @@ func (wfe *WebFrontEndImpl) Challenge(
 
 	authz, err := wfe.SA.GetAuthorization(ctx, authorizationID)
 	if err != nil {
-		// TODO(#1198): handle db errors etc
-		notFound()
+		if err == sql.ErrNoRows {
+			notFound()
+		} else {
+			wfe.sendError(response, logEvent, probs.ServerInternal("Problem getting authorization"), err)
+		}
 		return
 	}
 
diff --git a/wfe/wfe_test.go b/wfe/wfe_test.go
@@ -942,6 +942,27 @@ func TestChallenge(t *testing.T) {
 	test.AssertEquals(t, responseWriter.Code, http.StatusNotFound)
 	assertJSONEquals(t, responseWriter.Body.String(),
 		`{"type":"urn:acme:error:malformed","detail":"Expired authorization","status":404}`)
+
+	// Challenge Not found
+	challengeURL = ""
+	responseWriter = httptest.NewRecorder()
+	wfe.Challenge(ctx, newRequestEvent(), responseWriter,
+		makePostRequestWithPath(challengeURL,
+			signRequest(t, `{"resource":"challenge"}`, wfe.nonceService)))
+	test.AssertEquals(t, responseWriter.Code, http.StatusNotFound)
+	assertJSONEquals(t, responseWriter.Body.String(),
+		`{"type":"urn:acme:error:malformed","detail":"No such challenge","status":404}`)
+
+	// Unspecified database error
+	errorURL := "error_result/24"
+	responseWriter = httptest.NewRecorder()
+	wfe.Challenge(ctx, newRequestEvent(), responseWriter,
+		makePostRequestWithPath(errorURL,
+			signRequest(t, `{"resource":"challenge"}`, wfe.nonceService)))
+	test.AssertEquals(t, responseWriter.Code, http.StatusInternalServerError)
+	assertJSONEquals(t, responseWriter.Body.String(),
+		`{"type":"urn:acme:error:serverInternal","detail":"Problem getting authorization","status":500}`)
+
 }
 
 func TestBadNonce(t *testing.T) {
