From 4628ef8422c2118ee482dc15a65eedff5144d34c Mon Sep 17 00:00:00 2001
From: Adam Scarr <adam@flip.com.au>
Date: Wed, 25 Nov 2020 12:49:27 +1100
Subject: [PATCH] Dont hold error lock when calling into error presenters

This can result in a deadlock if error handling code calls GetErrors.
---
 graphql/context_response.go      |  5 +++--
 graphql/context_response_test.go | 13 +++++++++++++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/graphql/context_response.go b/graphql/context_response.go
index d4c2e75445c..c128fdb49ce 100644
--- a/graphql/context_response.go
+++ b/graphql/context_response.go
@@ -45,10 +45,11 @@ func AddErrorf(ctx context.Context, format string, args ...interface{}) {
 func AddError(ctx context.Context, err error) {
 	c := getResponseContext(ctx)
 
+	presentedError := c.errorPresenter(ctx, ErrorOnPath(ctx, err))
+
 	c.errorsMu.Lock()
 	defer c.errorsMu.Unlock()
-
-	c.errors = append(c.errors, c.errorPresenter(ctx, ErrorOnPath(ctx, err)))
+	c.errors = append(c.errors, presentedError)
 }
 
 func Recover(ctx context.Context, err interface{}) (userMessage error) {
diff --git a/graphql/context_response_test.go b/graphql/context_response_test.go
index aa300229c4e..acafe95aaec 100644
--- a/graphql/context_response_test.go
+++ b/graphql/context_response_test.go
@@ -79,3 +79,16 @@ func TestAddError(t *testing.T) {
 		})
 	}
 }
+
+func TestGetErrorFromPresenter(t *testing.T) {
+	ctx := WithResponseContext(context.Background(), func(ctx context.Context, err error) *gqlerror.Error {
+		errs := GetErrors(ctx)
+
+		// because we are still presenting the error it is not expected to be returned, but this should not deadlock.
+		require.Len(t, errs, 0)
+		return DefaultErrorPresenter(ctx, err)
+	}, nil)
+
+	ctx = WithFieldContext(ctx, &FieldContext{})
+	AddError(ctx, errors.New("foo1"))
+}