softwaremill / tapir

Rapid development of self-documenting APIs
https://tapir.softwaremill.com
Apache License 2.0
1.36k stars 418 forks source link

[BUG] emptyOutput problems #587

Closed DenisNovac closed 3 years ago

DenisNovac commented 4 years ago

Tapir version: 0.15

Describe the bug

Hello. I have this code:

val send: Endpoint[(Option[String], IncomingTextMessage), Unit, StatusCode, Nothing] =
  endpoint.post
      .in("send")
      .in(auth.apiKey(cookie[Option[String]]("sessionid")))
      .in(jsonBody[IncomingTextMessage])
      .out(statusCode)
      .errorOut(statusCode)
      .errorOut(
        oneOf(
          statusMapping(StatusCode.Forbidden, emptyOutput),
          statusMapping(StatusCode.InternalServerError, emptyOutput)
        )
     )

And the controller which does something like this:

def signIn(authMsg: Authorize): IO[Either[StatusCode, CookieValueWithMeta]] = {
    for {
      cookie <- AuthService.authorize(authMsg).attempt
    } yield cookie
  }.map(_.fold(
    {
      case e if e.getMessage.contains("more rows expected") =>
        logger.error(s"No user ${authMsg.id} found for authorization")
        StatusCode.Forbidden.asLeft[CookieValueWithMeta]
      case e =>
        logger.error(s"Unexpected exception on authentication with id ${authMsg.id}: \n$e")
        StatusCode.InternalServerError.asLeft[CookieValueWithMeta]
    },
    value => value.asRight[StatusCode]
  ))

In this implementation it gives just errors:

java.lang.IllegalArgumentException: No status code mapping for value: (), in output: OneOf(ArraySeq(StatusMapping(Some(403),Empty(sttp.tapir.Codec$$anon$5@4e5deb2e,Info(None,List(),false)),sttp.tapir.Tapir$$Lambda$630/0x000000084055a040@79719a6c), StatusMapping(Some(500),Empty(sttp.tapir.Codec$$anon$5@4e5deb2e,Info(None,List(),false)),sttp.tapir.Tapir$$Lambda$630/0x000000084055a040@407eeeb1)),sttp.tapir.Codec$$anon$5@7d12249)
    at sttp.tapir.server.internal.EncodeOutputs.$anonfun$applySingle$4(EncodeOutputs.scala:51)
    at scala.Option.getOrElse(Option.scala:201)
    at sttp.tapir.server.internal.EncodeOutputs.applySingle(EncodeOutputs.scala:51)
    at sttp.tapir.server.internal.EncodeOutputs.apply(EncodeOutputs.scala:14)
    at sttp.tapir.server.internal.EncodeOutputs.applyPair(EncodeOutputs.scala:30)
    at sttp.tapir.server.internal.EncodeOutputs.apply(EncodeOutputs.scala:16)
    at sttp.tapir.server.http4s.OutputToHttp4sResponse.apply(OutputToHttp4sResponse.scala:19)
    at sttp.tapir.server.http4s.EndpointToHttp4sServer.$anonfun$toRoutes$3(EndpointToHttp4sServer.scala:41)
    at scala.Function1.$anonfun$andThen$1(Function1.scala:85)
    at cats.effect.IO$Map.apply(IO.scala:1578)
    at cats.effect.IO$Map.apply(IO.scala:1576)
    at cats.effect.internals.IORunLoop$.cats$effect$internals$IORunLoop$$loop(IORunLoop.scala:145)
    at cats.effect.internals.IORunLoop$RestartCallback.signal(IORunLoop.scala:368)
    at cats.effect.internals.IORunLoop$RestartCallback.apply(IORunLoop.scala:387)
    at cats.effect.internals.IORunLoop$RestartCallback.apply(IORunLoop.scala:330)
    at cats.effect.internals.IORunLoop$.cats$effect$internals$IORunLoop$$loop(IORunLoop.scala:103)
    at cats.effect.internals.IORunLoop$RestartCallback.signal(IORunLoop.scala:366)
    at cats.effect.internals.IORunLoop$RestartCallback.apply(IORunLoop.scala:387)
    at cats.effect.internals.IORunLoop$RestartCallback.apply(IORunLoop.scala:330)
    at cats.effect.internals.IORunLoop$.cats$effect$internals$IORunLoop$$loop(IORunLoop.scala:141)
    at cats.effect.internals.IORunLoop$RestartCallback.signal(IORunLoop.scala:366)
    at cats.effect.internals.IORunLoop$RestartCallback.apply(IORunLoop.scala:387)
    at cats.effect.internals.IORunLoop$RestartCallback.apply(IORunLoop.scala:330)
    at cats.effect.internals.IOShift$Tick.run(IOShift.scala:36)
    at java.base/java.util.concurrent.ForkJoinTask$RunnableExecuteAction.exec(ForkJoinTask.java:1426)
    at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290)
    at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
    at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656)
    at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
    at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177)

I tried to change it like this so the value in mapping is not Unit:

val send: Endpoint[(Option[String], IncomingTextMessage), Unit, StatusCode, Nothing] =
  endpoint.post
      .in("send")
      .in(auth.apiKey(cookie[Option[String]]("sessionid")))
      .in(jsonBody[IncomingTextMessage])
      .out(statusCode)
      .errorOut(
        oneOf[StatusCode](
          statusMapping(StatusCode.Forbidden, emptyOutput),
          statusMapping(StatusCode.InternalServerError, emptyOutput)
        )
     )

It does not work either:

java.lang.IllegalArgumentException: No status code mapping for value: 500, in output: OneOf(ArraySeq(StatusMapping(Some(403),Empty(sttp.tapir.Codec$$anon$5@7af9092e,Info(None,List(),false)),sttp.tapir.Tapir$$Lambda$630/0x0000000840557040@b99ed34), StatusMapping(Some(500),Empty(sttp.tapir.Codec$$anon$5@7af9092e,Info(None,List(),false)),sttp.tapir.Tapir$$Lambda$630/0x0000000840557040@4e8a27ba)),sttp.tapir.Codec$$anon$5@2a97858e)
    at sttp.tapir.server.internal.EncodeOutputs.$anonfun$applySingle$4(EncodeOutputs.scala:51)
    at scala.Option.getOrElse(Option.scala:201)
    at sttp.tapir.server.internal.EncodeOutputs.applySingle(EncodeOutputs.scala:51)
    at sttp.tapir.server.internal.EncodeOutputs.apply(EncodeOutputs.scala:14)

Both implementations always returns 500. Also in second version of code there is no error codes in swagger interface.

Shouldn't the signature of emptyOutput be like "EndpointIO.Empty[Unit]"? It is not possible to reach description method through EndpointOutput type:

val emptyOutput: EndpointOutput[Unit] = EndpointIO.Empty(Codec.idPlain(), EndpointIO.Info.empty)
adamw commented 4 years ago

This looks suspicious:

      .errorOut(statusCode)
      .errorOut(
        oneOf(
          statusMapping(StatusCode.Forbidden, emptyOutput),
          statusMapping(StatusCode.InternalServerError, emptyOutput)
        )

you should either return the status code as a value from the code, or provide mappings based on the type of the value being returned.

If you'd like to document possible status codes, you can do it using .errorOut(statusCode.description(Forbidden, "..."))

DenisNovac commented 4 years ago

This looks suspicious:

Agree, but it is kinda works. At least i can see this codes in Swagger.

you should either return the status code as a value from the code, or provide mappings based on the type of the value being returned.

If you'd like to document possible status codes, you can do it using .errorOut(statusCode.description(Forbidden, "..."))

But how do i describe several status codes without statusMapping? If i do like this:

endpoint.post
      .in("auth")
      .in(jsonBody[Authorize])
      .out(setCookie("sessionid"))
      .tag("Auth")
      .summary("Sign in with user id and password")
      .errorOut(statusCode.description(StatusCode.Forbidden, "Invalid user ID or password"))
      .errorOut(statusCode.description(StatusCode.InternalServerError, "Unknown error"))

My signature becomes Endpoint[Authorize, (StatusCode, StatusCode), CookieValueWithMeta, Nothing]

adamw commented 4 years ago

Won't

.errorOut(statusCode
  .description(StatusCode.Forbidden, "Invalid user ID or password")
  .description(StatusCode.InternalServerError, "Unknown error"))

work? You need a single status code, otherwise tapir gets confused ;) That's a separate feature, that we should add, that there are no conflicting inputs/outputs int he endpoint description (that would be validated at run-time, when the description is created)

DenisNovac commented 4 years ago

Oh, now i understand. Chained description calls works, thank you. I still don't understand why emptyOutput fails though. Is there any other application to it except for oneOf with StatusMapping?

adamw commented 4 years ago

I think oneOf is the only application. Though the types in the branches should be distinct, so that the right status code can be picked depending on the outcome of the logic function.

The problem seems to be with multiple status outputs, but I would have to debug to say precisely what happened.

adamw commented 3 years ago

Closing as either the problem has been solved, or lacks details. Plus the oneOf design changed in 0.18.0-M1