-
-
Notifications
You must be signed in to change notification settings - Fork 790
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow configuration to exclude `"start marker...."' from exception messages #1151
Comments
This would really be useful. The new redaction message introduced in #1039 is a pretty big regression IMO, as it very obviously leaks internal implementation details. Being able to disable that would be great. |
@jamesagnew can you provide an example of the text that you find you objectionable? |
@pjfanning Sure thing - my conern is that JsonProcessingException now produces messages like:
The reference to a java constant means this isn't a great message to return to REST API clients backed by Jackson any more since they shouldn't be aware of how the service is implemented internally. |
@jamesagnew What about the previous message makes it any more or less parseable? I don't believe that it is ever recommended to return exception messages from 3rd party libs in your proprietary REST services. This is just my opinion but what you return in your REST APIs does not really concern me and I don't think we should be tailoring our code to suit your specific use case. If you want to return error details in your REST services, you can very easily code it to parse out the details you need from the new and old JsonProcessingException strings. Furthermore, there is an API to get the JSONLocation from the JsonProcessingException. I think you should use that instead. |
@pjfanning I wouldn't say either is more or less parseable, I'm just trying to return a helpful error message to clients who have supplied an invalid request, including as much helpful information as possible about what the error was. Previously we returned:
..which was a bit weird because of the "Source: UNKNOWN" but ultimately pretty usable. Now we include this note about an internal java constant and I think that crosses a line into things our users will have security concerns with (for context, I help maintain a library that embeds Jackson - so I'm just trying to anticipate complaints we'll get from our users).
I don't really get this - There is already a setting that controls whether to include the offending request data or to replace it with the word redacted ( |
Hmm, actually constructing my own message works sometimes but not other times. In the event of parsing a document with unexpected content after the final closing brace, even |
Yeah, Exception messages are meant to convey information to Developers NOT End Users. I would be supportive of further configurability, although filing this issue was perhaps bit pre-mature: @JooHyukKim thank you for doing this but going forward let's let developers request things so there is always someone who has clear idea of things they want. |
True, @pjfanning @cowtowncoder sound right. Should've thought through. Closing until then! (until someone makes full requests) |
@JooHyukKim you had best intentions and it's tricky to balance keeping track of things that are reported vs having tons of issues as backlog. So no harm. |
I shall file a separte request for the additional configurability. |
(originally from google groups discussion)
Original Request (word for word)
When getting an JSON exception, even with source redacted - I was wondering if it was possible to configure jackson to not even mention the source, i.e instead of getting;
just reporting:
I couldn't see anything in the source to possible configure that, so am looking at regexing it out myself, tho I'd rather not manually do this for all exceptions if possible - I could seem to see any way of just getting the base message, and line/column information from any exceptions to build up my own message from fields either.
Considerations / Suggestions
JacksonException
base hasgetOriginalMessage()
and is the base ofall Jackson-produced exceptions. Other than that, there's no functionality to support such.
ErrorReportConfiguration
may be considered to be the container class, to host such config.The text was updated successfully, but these errors were encountered: