Run Recovery middleware's error handler in a goroutine

[pjebs]

-Addresses:
#147 (comment)

[jszwedko]

@pjebs looks like there are some race conditions introduced by this.

I'd still like to see some benchmarks motivating this change, but I can plan on running some after the race conditions are addressed.

[pjebs]

Are you sure the race conditions were introduced by this change or were they already there?
I can't see how that one line of code introduced a race condition

[jszwedko]

@pjebs I'm unable to reproduce on master, but am able to consistently reproduce given your patch.

Given that you are adding an additional goroutine, I don't think it is too surprising that race conditions might be introduced.

[trevrosen]

Any movement on this? A quick scan indicates that it would solve the issue I filed in
#244

[jszwedko]

@trevrosen It looks like you opened #245 after this comment so I assume you realized this wouldn't help with #244, but let me know if you still think it would (I'm not seeing how).

I'm still not sure this PR is necessary, but I'm open to thoughts. It is possible to spin off a goroutine in the provided ErrorHandlerFunc if the user really wants to.

go test ./... -race is still showing race conditions as well, but looking more closely, they may be due to the way the logger is being mocked in the tests with a bytes.Buffer

[trevrosen]

Yeah I rescind my earlier comment. I'm not sure what I must've been looking at before, lol...

IMHO the PR isn't strictly necessary. The user can choose to go in their function as you mention, and running recovery handlers in their own goroutine is an opinion that I'd expect a small lib like Negroni to not have.