-
Notifications
You must be signed in to change notification settings - Fork 113
add initialization context #126
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
Changes from all commits
b89f220
39f2241
93bc100
28be2ee
fe320d2
38d7088
f21f4a3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,12 +21,14 @@ extension Lambda { | |
internal final class Runner { | ||
private let runtimeClient: RuntimeClient | ||
private let eventLoop: EventLoop | ||
private let allocator: ByteBufferAllocator | ||
|
||
private var isGettingNextInvocation = false | ||
|
||
init(eventLoop: EventLoop, configuration: Configuration) { | ||
self.eventLoop = eventLoop | ||
self.runtimeClient = RuntimeClient(eventLoop: self.eventLoop, configuration: configuration.runtimeEngine) | ||
self.allocator = ByteBufferAllocator() | ||
} | ||
|
||
/// Run the user provided initializer. This *must* only be called once. | ||
|
@@ -36,13 +38,22 @@ 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, | ||
allocator: self.allocator) | ||
return factory(context) | ||
// Hopping back to "our" EventLoop is importnant in case the factory returns a future | ||
// that originated from a foreign EventLoop/EventLoopGroup. | ||
// This can happen if the factory uses a library (let's say a database client) that manages its own threads/loops | ||
// for whatever reason and returns a future that originated from that foreign EventLoop. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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> { | ||
|
@@ -54,9 +65,17 @@ extension Lambda { | |
}.flatMap { invocation, event in | ||
// 2. send invocation to handler | ||
self.isGettingNextInvocation = false | ||
let context = Context(logger: logger, eventLoop: self.eventLoop, invocation: invocation) | ||
let context = Context(logger: logger, | ||
eventLoop: self.eventLoop, | ||
allocator: self.allocator, | ||
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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👀 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tomerd The only situation in which we would need to There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Vapor doesn't do a There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
.mapResult { result in | ||
if case .failure(let error) = result { | ||
logger.warning("lambda handler returned an error: \(error)") | ||
|
@@ -82,15 +101,16 @@ extension Lambda { | |
} | ||
|
||
private extension Lambda.Context { | ||
convenience init(logger: Logger, eventLoop: EventLoop, invocation: Lambda.Invocation) { | ||
convenience init(logger: Logger, eventLoop: EventLoop, allocator: ByteBufferAllocator, invocation: Lambda.Invocation) { | ||
self.init(requestID: invocation.requestID, | ||
traceID: invocation.traceID, | ||
invokedFunctionARN: invocation.invokedFunctionARN, | ||
deadline: DispatchWallTime(millisSinceEpoch: invocation.deadlineInMillisSinceEpoch), | ||
cognitoIdentity: invocation.cognitoIdentity, | ||
clientContext: invocation.clientContext, | ||
logger: logger, | ||
eventLoop: eventLoop) | ||
eventLoop: eventLoop, | ||
allocator: allocator) | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
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 calledinitialize