-
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 1 commit
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 |
---|---|---|
|
@@ -37,13 +37,19 @@ extension Lambda { | |
// 1. create the handler from the factory | ||
// 2. report initialization error if one occured | ||
let context = InitializationContext(logger: logger, eventLoop: self.eventLoop) | ||
return factory(context).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)") | ||
return factory(context) | ||
// hopping back to "our" EventLoop is importnant in case the factory returns | ||
// a future that originiated from a different EventLoop | ||
// this can happen if the factory uses a library (lets say a DB client) that manages it's own EventLoops | ||
// for whatever reason and returns a future that originated from that foreign EventLoop. | ||
tomerd marked this conversation as resolved.
Show resolved
Hide resolved
|
||
.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> { | ||
|
@@ -58,6 +64,10 @@ 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 the "our" EventLoop is importnant in case the handler returns | ||
// a future that originiated from a different EventLoop | ||
// this can happen if the handler uses a library (lets say a DB client) that manages it's own EventLoops | ||
// 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 { | ||
|
Uh oh!
There was an error while loading. Please reload this page.
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.
just added a comment (it was already hopping) the diff is mostly formatting