search

ring-clojure / ring

Clojure HTTP server abstraction
MIT License
3.75k stars 519 forks source link

Prevent 2nd call of handler on unhandled exception #365

Closed rsslldnphy closed 5 years ago

rsslldnphy commented 5 years ago

Since version 9.4.12.v20180830, Jetty's AbstractCachingHandler calls handle a second time when an exception is thrown. This is so handlers can render custom error pages.

However in ring this would normally be done elsewhere in middleware - so this causes the normal handler to be called a second time on unhandled error, which causes a duplication of any side-effects of the handler function.

The second time handle is called, it's caled with a DispatcherType of DispatcherType/ERROR - so this commit checks for this DispatcherType and skips calling the user's ring handler in this case. This leads to a generic Jetty error page being returned.

Async handlers don't seem to be affected in the same way, so I haven't made the same change there, but I've added a test case so that if the same thing does start happening, it will be caught.

Fixes #364.

weavejester commented 5 years ago

Thanks! Sorry for the delay, but Github doesn't seem to notify for changes to commits.

The commit message subject is now fine, but it looks like the rest of the commit message could use a little work. It could be more concise, needs to wrap at 72 characters, and because it's plaintext rather than markdown, backticks aren't used.

So perhaps instead:

Prevent 2nd call of handler on unhandled exception

Since version 9.4.12.v20180830, Jetty's AbstractCachingHandler calls
its handle method a second time when an exception is thrown. This
commit prevents Ring's handler function from being called twice as a
result of this change to Jetty.
rsslldnphy commented 5 years ago

no worries - updated

weavejester commented 5 years ago

Merged. Thanks for the patch!

darkleaf commented 5 years ago

When this patch will be released?