Skip to content

Commit

Permalink
Merge pull request #1399 from 99designs/prevent-possible-error-deadlock
Browse files Browse the repository at this point in the history
Dont hold error lock when calling into error presenters
  • Loading branch information
vektah authored Nov 30, 2020
2 parents 0e12bfb + 4628ef8 commit 34a442c
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 2 deletions.
5 changes: 3 additions & 2 deletions graphql/context_response.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
13 changes: 13 additions & 0 deletions graphql/context_response_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
}

0 comments on commit 34a442c

Please sign in to comment.