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

add initialization context #126

Merged
merged 7 commits into from
Jun 16, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions Sources/AWSLambdaRuntimeCore/Lambda.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ public enum Lambda {

/// `ByteBufferLambdaHandler` factory.
///
/// A function that takes a `EventLoop` and returns an `EventLoopFuture` of a `ByteBufferLambdaHandler`
public typealias HandlerFactory = (EventLoop) -> EventLoopFuture<Handler>
/// A function that takes a `InitializationContext` and returns an `EventLoopFuture` of a `ByteBufferLambdaHandler`
public typealias HandlerFactory = (InitializationContext) -> EventLoopFuture<Handler>

/// Run a Lambda defined by implementing the `LambdaHandler` protocol.
///
Expand Down Expand Up @@ -58,7 +58,7 @@ public enum Lambda {
/// - factory: A `ByteBufferLambdaHandler` factory.
///
/// - note: This is a blocking operation that will run forever, as its lifecycle is managed by the AWS Lambda Runtime Engine.
public static func run(_ factory: @escaping (EventLoop) throws -> Handler) {
public static func run(_ factory: @escaping (InitializationContext) throws -> Handler) {
self.run(factory: factory)
}

Expand All @@ -73,19 +73,19 @@ public enum Lambda {
// for testing and internal use
@discardableResult
internal static func run(configuration: Configuration = .init(), handler: Handler) -> Result<Int, Error> {
self.run(configuration: configuration, factory: { $0.makeSucceededFuture(handler) })
self.run(configuration: configuration, factory: { $0.eventLoop.makeSucceededFuture(handler) })
}

// for testing and internal use
@discardableResult
internal static func run(configuration: Configuration = .init(), factory: @escaping (EventLoop) throws -> Handler) -> Result<Int, Error> {
self.run(configuration: configuration, factory: { eventloop -> EventLoopFuture<Handler> in
let promise = eventloop.makePromise(of: Handler.self)
internal static func run(configuration: Configuration = .init(), factory: @escaping (InitializationContext) throws -> Handler) -> Result<Int, Error> {
self.run(configuration: configuration, factory: { context -> EventLoopFuture<Handler> in
let promise = context.eventLoop.makePromise(of: Handler.self)
// if we have a callback based handler factory, we offload the creation of the handler
// onto the default offload queue, to ensure that the eventloop is never blocked.
Lambda.defaultOffloadQueue.async {
do {
promise.succeed(try factory(eventloop))
promise.succeed(try factory(context))
} catch {
promise.fail(error)
}
Expand Down
27 changes: 27 additions & 0 deletions Sources/AWSLambdaRuntimeCore/LambdaContext.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,33 @@ import Dispatch
import Logging
import NIO

// MARK: - InitializationContext

extension Lambda {
/// Lambda runtime initialization context.
/// The Lambda runtime generates and passes the `InitializationContext` to the Lambda handler as an argument.
tomerd marked this conversation as resolved.
Show resolved Hide resolved
public final class InitializationContext {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

also considered InitContext but this feels more correct since the function is called initialize

/// `Logger` to log with
///
/// - note: The `LogLevel` can be configured using the `LOG_LEVEL` environment variable.
public let logger: Logger

/// The `EventLoop` the Lambda is executed on. Use this to schedule work with.
/// This is useful when implementing the `EventLoopLambdaHandler` protocol.
///
/// - note: The `EventLoop` is shared with the Lambda runtime engine and should be handled with extra care.
/// Most importantly the `EventLoop` must never be blocked.
public let eventLoop: EventLoop
tomerd marked this conversation as resolved.
Show resolved Hide resolved

internal init(logger: Logger, eventLoop: EventLoop) {
self.eventLoop = eventLoop
self.logger = logger
}
}
}

// MARK: - Context

extension Lambda {
/// Lambda runtime context.
/// The Lambda runtime generates and passes the `Context` to the Lambda handler as an argument.
Expand Down
24 changes: 18 additions & 6 deletions Sources/AWSLambdaRuntimeCore/LambdaRunner.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,20 @@ extension Lambda {
logger.debug("initializing lambda")
// 1. create the handler from the factory
// 2. report initialization error if one occured
return factory(self.eventLoop).hop(to: self.eventLoop).peekError { error in
self.runtimeClient.reportInitializationError(logger: logger, error: error).peekError { reportingError in
// We're going to bail out because the init failed, so there's not a lot we can do other than log
// that we couldn't report this error back to the runtime.
logger.error("failed reporting initialization error to lambda runtime engine: \(reportingError)")
let context = InitializationContext(logger: logger, eventLoop: self.eventLoop)
return factory(context)
// Hopping back to "our" EventLoop is importnant in case the factory returns a future
// that originiated from a foreign EventLoop/EventLoopGroup.
tomerd marked this conversation as resolved.
Show resolved Hide resolved
// This can happen if the factory uses a library (lets say a DB client) that manages its own threads/loops
tomerd marked this conversation as resolved.
Show resolved Hide resolved
// for whatever reason and returns a future that originated from that foreign EventLoop.
Copy link
Contributor Author

@tomerd tomerd Jun 13, 2020

Choose a reason for hiding this comment

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

just added a comment (it was already hopping) the diff is mostly formatting

.hop(to: self.eventLoop)
.peekError { error in
self.runtimeClient.reportInitializationError(logger: logger, error: error).peekError { reportingError in
// We're going to bail out because the init failed, so there's not a lot we can do other than log
// that we couldn't report this error back to the runtime.
logger.error("failed reporting initialization error to lambda runtime engine: \(reportingError)")
}
}
}
}

func run(logger: Logger, handler: Handler) -> EventLoopFuture<Void> {
Expand All @@ -57,6 +64,11 @@ extension Lambda {
let context = Context(logger: logger, eventLoop: self.eventLoop, invocation: invocation)
logger.debug("sending invocation to lambda handler \(handler)")
return handler.handle(context: context, event: event)
// Hopping back to "our" EventLoop is importnant in case the handler returns a future that
// originiated from a foreign EventLoop/EventLoopGroup.
// This can happen if the handler uses a library (lets say a DB client) that manages its own threads/loops
// for whatever reason and returns a future that originated from that foreign EventLoop.
.hop(to: self.eventLoop)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

👀

Copy link
Member

Choose a reason for hiding this comment

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

@tomerd The only situation in which we would need to hop is if developers bring their own EventLoopGroup. Is that something that is likely?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this can happen is if they use some library (lets say a DB client) that manages it's own ELG for whatever reason and they return a future that originated from that ELG. while this is not a pattern we encourage, we also can't stop anyone from doing it so to be safe we should hop imo

Copy link
Member

Choose a reason for hiding this comment

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

Vapor doesn't do a hop in its routes and quite a number of people hit that issue at some point. After that they know its an issue and don't do it again. There is an argument for education here instead of protecting against. But at the same time a hop onto the same EventLoop is hardly a performance hit.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just as a reference: Where ever NIO takes a future (not that many APIs outside of bootstraps), it'll hop it to the correct event loop.

.mapResult { result in
if case .failure(let error) = result {
logger.warning("lambda handler returned an error: \(error)")
Expand Down
2 changes: 1 addition & 1 deletion Tests/AWSLambdaRuntimeCoreTests/Lambda+StringTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ class StringLambdaTest: XCTestCase {
typealias In = String
typealias Out = String

init(eventLoop: EventLoop) throws {
init(context: Lambda.InitializationContext) throws {
throw TestError("kaboom")
}

Expand Down
4 changes: 2 additions & 2 deletions Tests/AWSLambdaRuntimeCoreTests/LambdaRuntimeClientTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class LambdaRuntimeClientTest: XCTestCase {

func testBootstrapFailure() {
let behavior = Behavior()
XCTAssertThrowsError(try runLambda(behavior: behavior, factory: { $0.makeFailedFuture(TestError("boom")) })) { error in
XCTAssertThrowsError(try runLambda(behavior: behavior, factory: { $0.eventLoop.makeFailedFuture(TestError("boom")) })) { error in
XCTAssertEqual(error as? TestError, TestError("boom"))
}
XCTAssertEqual(behavior.state, 1)
Expand Down Expand Up @@ -186,7 +186,7 @@ class LambdaRuntimeClientTest: XCTestCase {
.failure(.internalServerError)
}
}
XCTAssertThrowsError(try runLambda(behavior: Behavior(), factory: { $0.makeFailedFuture(TestError("boom")) })) { error in
XCTAssertThrowsError(try runLambda(behavior: Behavior(), factory: { $0.eventLoop.makeFailedFuture(TestError("boom")) })) { error in
XCTAssertEqual(error as? TestError, TestError("boom"))
}
}
Expand Down
10 changes: 5 additions & 5 deletions Tests/AWSLambdaRuntimeCoreTests/LambdaTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class LambdaTest: XCTestCase {

var initialized = false

init(eventLoop: EventLoop) {
init(context: Lambda.InitializationContext) {
XCTAssertFalse(self.initialized)
self.initialized = true
}
Expand All @@ -72,7 +72,7 @@ class LambdaTest: XCTestCase {
XCTAssertNoThrow(try server.start().wait())
defer { XCTAssertNoThrow(try server.stop().wait()) }

let result = Lambda.run(factory: { $0.makeFailedFuture(TestError("kaboom")) })
let result = Lambda.run(factory: { $0.eventLoop.makeFailedFuture(TestError("kaboom")) })
assertLambdaLifecycleResult(result, shouldFailWithError: TestError("kaboom"))
}

Expand All @@ -85,7 +85,7 @@ class LambdaTest: XCTestCase {
typealias In = String
typealias Out = Void

init(eventLoop: EventLoop) throws {
init(context: Lambda.InitializationContext) throws {
throw TestError("kaboom")
}

Expand Down Expand Up @@ -124,7 +124,7 @@ class LambdaTest: XCTestCase {
XCTAssertNoThrow(try server.start().wait())
defer { XCTAssertNoThrow(try server.stop().wait()) }

let result = Lambda.run(factory: { $0.makeFailedFuture(TestError("kaboom")) })
let result = Lambda.run(factory: { $0.eventLoop.makeFailedFuture(TestError("kaboom")) })
assertLambdaLifecycleResult(result, shouldFailWithError: TestError("kaboom"))
}

Expand All @@ -143,7 +143,7 @@ class LambdaTest: XCTestCase {
usleep(100_000)
kill(getpid(), signal.rawValue)
}
let result = Lambda.run(configuration: configuration, factory: { $0.makeSucceededFuture(EchoHandler()) })
let result = Lambda.run(configuration: configuration, factory: { $0.eventLoop.makeSucceededFuture(EchoHandler()) })

switch result {
case .success(let invocationCount):
Expand Down
2 changes: 1 addition & 1 deletion Tests/AWSLambdaRuntimeCoreTests/Utils.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import NIO
import XCTest

func runLambda(behavior: LambdaServerBehavior, handler: Lambda.Handler) throws {
try runLambda(behavior: behavior, factory: { $0.makeSucceededFuture(handler) })
try runLambda(behavior: behavior, factory: { $0.eventLoop.makeSucceededFuture(handler) })
}

func runLambda(behavior: LambdaServerBehavior, factory: @escaping Lambda.HandlerFactory) throws {
Expand Down