Include exception message and backtrace in response#524
Include exception message and backtrace in response#524lgebhardt merged 1 commit intoJSONAPI-Resources:masterfrom
Conversation
|
Ah good call. The meta key would probably suffice. Interestingly since I created this PR I've been tailing |
|
@iamvery I see it as worthwhile. It's sometimes slightly difficult to tail a log on a qa server. |
|
👍 I'll try to get an update in soon. Thanks for the feedback! |
|
Just chiming in here to support using |
|
❤️ |
iamvery
force-pushed
the
exception-response
branch
from
a180bbe to
fab8c0e
Compare
|
I just pushed an update that moves this information to |
|
@iamvery Thanks for patching this up. But I have a few suggestions. I want to avoid having the meta fields output if they are nil. I also think by passing the No need to do this now, but I'm thinking I may want to create a configuration option to control whether these are output. I haven't decided if an option is overkill at this point. |
* When in a non-prod environment it is useful to include more information about the error for the purpose of debugging. Imagine you are test-driving an acceptance test and you hit a 500 error. There would be no way to tell what caused the problem unless you tailed log/test.log. Similar to Rails' special error pages in non-prod environments, this gives you a little more context in the response. * It is important, however, that this information not be included in production. Doing so might leak internal details about your app a create a security concern.
iamvery
force-pushed
the
exception-response
branch
from
fab8c0e to
81c05a1
Compare
|
Thanks for the feedback! The PR has been updated. I'm thinking that meta being |
|
Thanks @iamvery. The errors do not run through that code. They are actually just output as a hash in https://github.com/cerebris/jsonapi-resources/blob/master/lib/jsonapi/response_document.rb#L102 so the nil parameters come through as nils. So by adding |
Include exception message and backtrace in response
|
Shouldn't it be |
|
Hah! Good catch. Tests, amitite? I'll send a patch unless someone else gets to it 😆 |
4 participants
Following #502, this is a simple potential solution to communicating exception information in non-prod.
It bothers me a bit to submit a change that doesn't have a test, but wasn't clear where such a test would exist as the response of internal server errors doesn't appear to be tested. To that end, this is something of a development support feature and presumably as long as it doesn't break production functionality it should be safe to ship.
TIL
One thing I did realize while I was working on this is that the backtrace produced by the error is logged to
log/test.log. So one could conceivably monitor this file if they encountered an unexpected error in test. I must have failed to realize this before simple because it's not my typical workflow. Maybe that's not true for others?So, what do you think? Is this useful?