ninjaframework / ninja

Ninja is a full stack web framework for Java. Rock solid, fast and super productive.
http://www.ninjaframework.org
Apache License 2.0
1.91k stars 518 forks source link

BadRequestException accepts String detail message but it's not used or log anywhere #304

Closed liborjelinek closed 6 years ago

liborjelinek commented 9 years ago

It would be nice if exception message sent to BadRequest(String message) is not lost but logged and passed to 400badRequest.ftl.thml template.

I'm Ninja for just about 1 hour so far and still dont' understend rending enough but I suggest to change .render() (highlighted) in NinjaDefault.getBadRequestResult(Context, Exception)

@Override
public Result getBadRequestResult(Context context, Exception exception) {

    String messageI18n 
            = messages.getWithDefault(
                    NinjaConstant.I18N_NINJA_SYSTEM_BAD_REQUEST_TEXT_KEY,
                    NinjaConstant.I18N_NINJA_SYSTEM_BAD_REQUEST_TEXT_DEFAULT,
                    context,
                    Optional.<Result>absent());

    Message message = new Message(messageI18n); 

    Result result = Results
            .badRequest()
            .supportedContentTypes(Result.TEXT_HTML, Result.APPLICATION_JSON, Result.APPLICATION_XML)
            .fallbackContentType(Result.TEXT_HTML)
    >>>>>>  .render(message)   <<<<<<<<
            .template(NinjaConstant.LOCATION_VIEW_FTL_HTML_BAD_REQUEST);

    return result;

to something like .render(Message).render("exception", exception)?

Also if accepted, docs should be updated accordingly.

raphaelbauer commented 9 years ago

I'd say rendering the exception message is not necessarily something the end-user has to see. But logging is important - are you sure that the exception is not logged right now?

jjlauer commented 9 years ago

Hey @raphaelbauer -- I think some of the work I've been putting in on a "Diagnostic" mode enhancement for Dev mode may help @bircow

It's mostly while in dev mode that I find more detailed error messages about what went wrong nice to have. When not in dev mode then you want to actually display the system template for the particular error.

See https://github.com/ninjaframework/ninja/pull/324 for some of my ideas

jlannoy commented 6 years ago

Fixed. Proposed PR #627