I wonder if we should expose this as e.g.

trait LambdaRuntimeEnv[F] {
  def lambdaRuntimeDir: F[String] = ???
  // ...
}

object LambdaRuntimeEnv {
  def apply[F[_]](env: Env[F]): LambdaRuntimeEnv[F] = ???
}

Hi, I'm not sure I 100% understand this part. Isn't the Env[F] from cats.effect.std not included in the current version of cats effect? Or do you mean that I should create a custom Env[F] for the moment until it becomes available?

Also, I'm not sure how I would implement the methods within the LambdaRuntimeEnv[F] trait. I would have assumed that the implementations of each of the methods would be given here

object LambdaRuntimeEnv {
	def apply[F[_]](env: Env[F]): LambdaRuntimeEnv[F] = new LambdaRuntimeEnv[F] {
		def lambdaRuntimeDir: F[String] = ???
		// ...
	}
}

instead of giving implementations directly within the trait.

Isn't the Env[F] from cats.effect.std not included in the current version of cats effect?

It is now available in v3.4.0-RC1, you can update to that and use it :)

Also, I'm not sure how I would implement the methods within the LambdaRuntimeEnv[F] trait.

Sorry, I typed that out too hasty :) you are right, the implementations should go in the apply method as you wrote. Also I think I meant to write apply[F[_]](implicit env: Env[F]) ...

yes :) https://github.com/cquiroz/scala-java-time

Instead of Instant.now you can use F.realTime. This is better for tests/mocking as well.

Looking at it again, I think I made a mistake, sorry! We don't want .background here, we should just return the result of foreverM maybe 🤔 leave it up to the caller to background or not. This way we can properly raise an error (with .background any error would be swallowed).

I have some memory these can just be parsed from JSON.

The Right:tm: way to do this is to define our own header models for each of the AWS headers. Take your pick from examples :)

https://github.com/http4s/http4s/tree/v0.23.16/core/shared/src/main/scala/org/http4s/headers

I had a go at creating header models using a similar way how the headers defined in the headers package are defined, but they use functions that are private to the http4s package. Is it possible to create header models not using these private functions?

Which private methods specifically?

The main one was the ParseResult.fromParser method which seems needed to create an instance of Header. An example of its use is in https://github.com/http4s/http4s/blob/v0.23.16/core/shared/src/main/scala/org/http4s/headers/Accept-Post.scala.

edit: maybe I didn't have a close enough look at other files, there seems to be other ways of defining a ParseResult so I'll have another go.

I think you can just inline that from here, it's pretty straightforward.
https://github.com/http4s/http4s/blob/14afe7a41ee66ef81af4861269b3f574cd9aea8c/core/shared/src/main/scala/org/http4s/MessageFailure.scala#L84-L88

I suspect the try/catch is no longer necessary, it is an artifact of old code 😂

We should prefer class instead of case class since that is better for binary-compatibility, unfortunately. Indeed, I'm in the process of fixing this across the entire project 😕

We can just directly request an implicit env: LambdaRuntimeEnv[F]

It's a bit superstitious, but would you mind if we avoid using the DSL here and just use the Request(...) constructor? Since our use is so small and straightforward, and DSLs always risk to be more unstable than the core APIs (indeed, there was some discussion of moving the routing DSL to a separate repository).

These can all be private[this]

We should avoid .get, instead something like this with MonadThrow[F].

Hmm, on second thought, I think we want:

successful returns a Boolean, which is currently being ignored. I wonder if expect[Unit] will work, so it will raise an exception on failure.

Instead of converting to millis and back to duration, you can use F.realTime and do that math directly with FiniteDuration.

huh, does using uri"..." here actually work? That seems like a bug 😅 since it has no way of proving at compile-time that api is valid.

Woops :) thanks! can also rename the file to just file("lambda-runtime")

I think we can make this Temporal.

We should avoid the unsafe here, and use liftTo[F] instead.

No, we should not raise a CancellationException. That will stop the entire Lambda, just because of processing of one event was self-canceled :)

The idea behind the running the handler on a separate fiber, and then joining that fiber to check the outcome, is so that if the handler self-cancels for a particular event, the Lambda can continue processing additional events.

Instead of Sync I think we can ask for FlatMap and Clock.

Eh, since it's a private method you can just re-use the Temporal here.

response.as is not DSL, it's a core method :)

By DSL, I very specifically meant things you need to import from org.http4s.client.dsl._

Btw, thanks for raising the issue about loading the handler function. After thinking about it, you are right that we need a small adjustment here actually.

Instead of requesting the handler, we should request a Resource that is used to acquired the handler. This way, if there are any errors during that process, we can log them correctly. It also leaves the door open for someone to implement dynamic loading, but we shouldn't worry about that here.

I am still a bit confused on where initialization errors should be handled. It seems like using handleErrorWith on the result of handler.use(...) would pick up errors that could have occurred at any point, including per-request errors. I checked the C++ and Rust runtimes and from what I can see they don't seem to use the initialization error URL at all.

Good point. I think the initializer errors should be caught and handled right before the use. If you've made it to the use, I think it's safe to say that you've initialized successfully :)

I checked the C++ and Rust runtimes and from what I can see they don't seem to use the initialization error URL at all.

Interesting. What about the JVM or JS runtimes?

I believe the JVM runtime uses reflection based loading of the handler class and the JS runtime uses dynamic loading. Errors can occur during these steps so that's where the initialization error URL is used.

I think we can do Uri.fromString(s"/$api/$ApiVersion/runtime/") once, and then safely build all the other URIs using that.

I don't think we want to raiseErrors here, since that will crash the Lambda.

When we join the fiber, if there is an error or cancellation we should send it to the invocation error URL and suppress it.

My thinking was that these potential errors would get caught in the handleErrorWith method below and instead of crashing the lambda, it would send a invocation error to the invocation error URL. Although I haven't actually tested that this works yet and will need to add another test to see if it can process multiple requests with failures.

My reasoning in changing the existing code (using Option[Json] as the type for result) to instead raising errors, was so that failures in places such as creating the context could be caught properly and handled as an invocation error. Previously, this wasn't the case and context errors would crash the lambda.

Yes, sorry, you are right, I read that much too hastily :) indeed, the handleErrorWith will catch these errors. It looks like it will also catch an error POSTing a response.

I guess there could be a distinction between handler errors (since these occur in userland) and errors that occur outside/after the handler, since I assume those would be our fault (or an issue in the runtime)? I actually don't know what the response URL returns if the user handler returns a Json that is invalid for this particular Lambda.

Do we know if these values ever change while the process is running? If not maybe we should load them from the Env once, and keep them cached.

I cannot imagine why they'd change, but I know Lambda does rely on other envs changing at runtime, so it wouldn't be surprising.

Ah this might have been my mistake. The custom lambda runtime docs mention that reading environment variables is an initialization task while the custom runtime is currently reading them for every invocation. So no, I don't think they will change so I think it is safe to read them once during initialization.

Thanks! No not at all, I think it was my bad suggestion in #276 (comment) 😅 sorry!

So then let's refactor this so they are read once during initialization.

Yep, I can do that if you haven't already started?

Yes, please go ahead!

Can I ask why Nothing is used instead of Unit in places such as this? With this code, the compiler complains with

type mismatch;
 found   : Either[Throwable,feral.lambda.runtime.LambdaSettings] => F[Nothing]
 required: Either[Throwable,feral.lambda.runtime.LambdaSettings] => F[B]

but runs fine with Unit

Good question. Technically we should return Nothing here because these methods never return—they start a process that runs forever, and will never produce an output. But perhaps type inference is too uncooperative for this to be practical 🤔

For example, see Resource#useForever:
https://github.com/typelevel/cats-effect/blob/52b5a3ba1623ff40fc48318c9a0bfae558f0e5ed/kernel/shared/src/main/scala/cats/effect/kernel/Resource.scala#L217-L224

I think you @scott-thomson239 should be able to use Nothing (and should use it, because, as @armanbilge says, it's the right thing to do).
But you probably need to tweak the variance of F[_]. My hunch is that it should be declared as F[+_], so

private[this] def handleInitError[F[+_]](

and similarly for others.
Good luck 🤞