Skip to content

Conversation

@potomak
Copy link

@potomak potomak commented Apr 2, 2013

I've added exception handling in middleware process execution to postprocess also error responses.

Closes #46

@knutin
Copy link
Owner

knutin commented Apr 4, 2013

Hi, thanks for the patch!

Your implementation is a bit different than what I had in mind.

In the branch request_id I have added an id to each request that is also available in the request_{throw,error,exit} events. The request id will be sent to the browser, much like what Varnish does. https://github.com/wooga/elli_access_log can use this request id. Or if you have your own error logging middleware that for example logs to a database, you can use the request id as primary key.

What do you think?

@potomak
Copy link
Author

potomak commented Apr 4, 2013

I didn't see the request_id branch!
That makes a lot of sense, without the elli_request:id I just used the elli_request:to_proplist as you are using in https://github.com/wooga/elli_access_log/blob/master/src/elli_access_log.erl#L81.

The only tricky part of this pull request is the exception handling in elli_middleware.erl that makes always run the postprocess method post-processing the response.

What do you think?

I'd be happy to update the middleware to follow your specs, is it ok for you?

@potomak
Copy link
Author

potomak commented Apr 4, 2013

See #62

@potomak potomak closed this Apr 4, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Better 4xx, 5xx responses

2 participants