ktorio / ktor

Framework for quickly creating connected applications in Kotlin with minimal effort
https://ktor.io
Apache License 2.0
13.05k stars 1.07k forks source link

Inconsistent source of status for ApplicationCall#respond variants #723

Open DerkSchooltink opened 6 years ago

DerkSchooltink commented 6 years ago

Given the following code:

call.respond(HttpStatusCode.InternalServerError, "{}") call.respondText("{}", ContentType.Application.Json, HttpStatusCode.InternalServerError)

I am intercepting the response in the 'after' of the 'sendPipeline':

pipeline.sendPipeline.intercept(ApplicationSendPipeline.After) {
           println("Respond: " + context.response.status())
           println("Respond TextContent: " + (this.subject as? TextContent)?.status)
}

The results for this interceptor are as following:

Usecase Code ApplicationResponse#status() TextContent#status
1 call.respond(HttpStatusCode.InternalServerError, "{}") 500 Internal Server Error null
2 call.respondText("{}", ContentType.Application.Json, HttpStatusCode.InternalServerError) null 500 Internal Server Error

Perhaps it is logical that usecase 1 has a null value for TextContent#status because it is not an actual TextContent. However, I would expect for usecase 2 that ApplicationResponse#status() would be set.

tl;dr: There is no single source of truth for getting the statuscode (which I would expect to be ApplicationResponse#status().

tajobe commented 5 years ago

FWIW, if you wanted one place, it seems like it can always be retrieved from the subject as OutgoingContent after the Render phase:

        /**
         * Phase to render any current pipeline subject into [io.ktor.http.content.OutgoingContent]
         *
         * Beyond this phase only [io.ktor.http.content.OutgoingContent] should be produced by any interceptor
         */
        val Render = PipelinePhase("Render")

The caveat is that it can still be null if no status code was provided in a respond(...) (as in, doesn't get defaulted to 200). That said, this information feels like it certainly should be available in the response API.

oleg-larshin commented 4 years ago

Please check the following ticket on YouTrack for follow-ups to this issue. GitHub issues will be closed in the coming weeks.