Skip to content

Alternative Ktor Raise DSL #3581

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

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

tKe
Copy link
Collaborator

@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
Collaborator 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
Collaborator 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
tKe and others added 8 commits February 18, 2025 23:21
I'm wary `Response` is potentially too non-specific, however
`RoutingResponse` clashes with `io.ktor.server.routing.RoutingResponse`
using the right one relies on ensuring the right import is in, or
 qualifying the parameter
includes a delegate based access for query/path parameters
allowing for:
`val id: Int by pathRaising`
`val userId: Int by queryRaising("user-id")"`
`val nameUpper: String by pathRaising { it.uppercase() }`

`validate { ... }` block using RaiseAccumulate has `by pathAccumulating` and `by queryAccumulating` forms and a `receiveAccumulating<T>()` helper.

This makes for a nice and succinct:
```kotlin
putOrRaise("/user/{name}") {
  val person = validate {
    val name: String by pathAccumulating
    val age: Int by queryAccumulating
    val info: Info by receiveAccumulating()
    Person(name, age, info)
  }
  Created(person)
}
```

There's also a Ktor Route-scoped plugin to provide the default error responses:

```kotlin
install(RaiseErrorResponse) {
  errorResponse = { validationError(it.nel()) }
  errorsResponse = { validationError(it) }
}
```
@tKe
Copy link
Collaborator Author

tKe commented Feb 28, 2025

I've included a delegate based access for query/path parameters allowing for:

val id: Int by pathRaising
val userId: Int by queryRaising("user-id")"
val nameUpper: String by pathRaising { it.uppercase() }

validate { ... } block using RaiseAccumulate has by pathAccumulating and by queryAccumulating forms and a receiveAccumulating<T>() helper.

This makes for a nice and succinct:

putOrRaise("/user/{name}") {
  val person = validate {
    val name: String by pathAccumulating
    val age: Int by queryAccumulating
    val info: Info by receiveAccumulating()
    Person(name, age, info)
  }
  Created(person)
}

There's also a Ktor Route-scoped plugin to provide the default error responses:

install(RaiseErrorResponse) {
  errorResponse = { validationError(it.nel()) }
  errorsResponse = { validationError(it) }
}

@tKe tKe marked this pull request as ready for review February 28, 2025 21:44
tKe added 3 commits February 28, 2025 22:16
only need one error response type - a single request error can be
wrapped in nel
import kotlin.properties.ReadOnlyProperty
import kotlin.reflect.KProperty

public abstract class RaisingParameterProvider internal constructor(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 at the moment this only raises when a delegated property is used.
Do we want to raise as soon as a delegate is acquired?

This could be achieved by the same process as the accumulating delegate provider but returning an already-resolved delegate.

tKe added 2 commits March 20, 2025 18:32
…row-ktor-api

# Conflicts:
#	arrow-libs/integrations/arrow-raise-ktor-server/api/arrow-raise-ktor-server.klib.api
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