Skip to content
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

Alternative Ktor Raise DSL #3581

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

tKe
Copy link
Contributor

@tKe tKe commented Feb 10, 2025

Not complete by any stretch but hopefully enough to start any discussion.

@serras
Copy link
Member

serras commented Feb 12, 2025

@tKe please don't take the merge of #3576 as making the new Ktor module the "correct" one, the door is still open to the changes you propose.

Would you mind elaborating how this PR is different from the other one about Ktor + Raise, and the pros/cons of this implementation?

@tKe
Copy link
Contributor Author

tKe commented Feb 12, 2025

At the moment, this PR is only addressing one "con" of the existing API, which is the reliance on OutgoingContent (or HttpStatusCode) as the only "raisable" responses.

Raising OutgoingContent

The downside of only allowing for OutgoingContent within Ktor is that it shortcircuits any ContentNegotiation or response transformer plugins.

An example where this is an issue would be if a service was intended to have a typed/structured error response, and desired to use the standard approach of using call.respond(HttpStatus.BadRequest, myErrorPayload) when an error response is desired. This wouldn't be possible via raise(OutgoingContent) without separately encoding the error payload into a TextContent or other OutgoingContent type.

@Serializable
data class ErrorPayload(val code: String, val detail: String)

private fun Raise<OutgoingContent>.handleError(domainError: DomainError): Nothing {
  val errorPayload = when(domainError) {
    is UserBanned -> ErrorPayload(
      code = "USER_BANNED", 
      detail = "user ${domainError.userId} was banned because ${domainError.reason}"
    )
    else -> TODO()
  }
  val outgoingContent = // convert via JSON to byte array?
  raise(outgoingContent)
}

fun Application.module(userService: UserService) {
  install(ContentNegotiation) { json() }
  routing {
    getOrRaise("/users/{userId}") {
      val userId = // get from path or raise
      
      val user = withError(::handleError) { 
        userService.lookup(userId) ?: raise(HttpStatusCode.NotFound)
      }

      call.respond(user)
    }
  }
}

The desirable API would allow for raising the errorPayload, potentially with a specific status code, and let the ktor plugins handle conversion to outgoing content as you would have if you had used call.respond, while still allowing for the short-circuiting of raise.

private fun Raise<RoutingResponse>.handleError(domainError: DomainError): Nothing {
  val errorPayload = when(domainError) {
    is UserBanned -> raise(HttpStatusCode.Conflict, ErrorPayload(
      code = "USER_BANNED", 
      detail = "user ${domainError.userId} was banned because ${domainError.reason}"
    ))
    else -> TODO()
  }
  val outgoingContent = // convert via JSON to byte array? how to determine status code?
  raise(outgoingContent)
}

I've also attempted to avoid context receivers for implementing this, and have instead defined extensions for a Raise<RoutingResponse> for handling HttpStatusCode and OutgoingContent responses - that said, this is just syntactic sugar and doesn't actually expose a Raise<HttpStatusCode> or Raise<OutgoingContent> - union types would be lovely here...

Composability with ktor APIs

This PR also attempts to implement error/raise response handling in a handleOrRaise as an analogue of ktor's handle generic route handler, rather than only specific ktor get or post style route handlers (which it admittedly could also do for ease-of-use).

handleOrRaise allows for "breaking out" of a handler with a response (either empty with HttpStatusCode, a pre-converted OutgoingContent, or a reified payload that would flow through the ktor plugins, e.g ContentNegotiation).

Two paths to a response

The handleOrRaise still relies on explicitly calling call.response for the "happy" response, as would be expected in a standard ktor route handler, whereas the raise response is automatically invoking call.respond for you.

To approach this, I've attempted to provide a mechanism in which the "happy-path" call.response is also achieved automatically with the successful result of the lambda/body - this would ensure that call.response is always called at least once, either through the raise path or the "happy" path.

It might be desirable for these respondOrRaise-style to discourage direct use of call.respond through compile-time warnings, although this is might prove difficult for the extension methods such as respondText.

A start on this is this is attempted in respondOrRaise.kt with an example (and alternative) raisingGet implementation.

There is also a RoutingContext.respondOrRaise that can be composed inside existing ktor get(...) { ... } or post(...) { ... } style handlers, e.g.

routing {
  get("/users/{userId}") {
    respondOrRaise {
      val userId = pathParameters["userId"]
      withError({ raise(convertToError(it)) }) {
        userService.lookupUser(userId) ?: raise(HttpStatusCode.NotFound)
      }
    }
  }
}

validate et al

I've not (yet) considered the validate DSL and/or pathOrRaise/queryOrRaise components of the original API, but they would likely warrant aligning to a similar approach. The only thing this PR does with those is remove the duplicate of the RaiseOrAccumulate that is now part of arrow core.

existing getOrRaise/putOrRaise extensions

I've currently left the existing getOrRaise/putOrRaise extensions in routing.kt for now.

I would intend to replace them with extensions that either:

  • require explicit happy-path call.response like the non-raising ktor API (equivalent to route(path, HttpMethod.Get) { handleOrRaise { body() } })
  • implicitly call call.respond with the happy-path result (equivalent to route(path, HttpMethod.Get) { respondOrRaise { body() } })

note: raisingGet is an example of the latter but with an alternative name to avoid clashing while this PR is elaborated

tKe added 2 commits February 12, 2025 19:29
…row-ktor-api

# Conflicts:
#	arrow-libs/integrations/arrow-raise-ktor-server/build.gradle.kts
@tKe
Copy link
Contributor Author

tKe commented Feb 13, 2025

I've now replaced the existing verbOrRaise helpers with ones that will implicitly respond with the result of the body lambda.

You could now achieve the above example with something like:

routing {
  getOrRaise("/users/{userId?}") {
    val userId = call.pathParameters["userId"] ?: raiseBadRequest("userId not specified")
    withError(::handleError) {
      userService.lookupUser(userId) ?: raise(HttpStatusCode.NotFound)
    }
  }
}

@serras serras requested a review from nomisRev February 13, 2025 20:43
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.

2 participants