Skip to content

add an option to start the local debugging server based on an env variable #87

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Jun 1, 2020
Merged

add an option to start the local debugging server based on an env variable #87

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
18a8bb7
add an option to start the local debugging server based on an env var…
tomerd May 27, 2020
c761c44
Merge branch 'master' into feature/easier-local-debugging
tomerd May 27, 2020
83861da
Merge branch 'master' into feature/easier-local-debugging
tomerd May 28, 2020
f3dc137
Merge branch 'master' into feature/easier-local-debugging
tomerd Jun 1, 2020
4115008
fixup
tomerd Jun 1, 2020
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
10 changes: 5 additions & 5 deletions Examples/LocalDebugging/MyLambda/Sources/MyLambda/main.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
import AWSLambdaRuntime
import Shared

try Lambda.withLocalServer {
Lambda.run { (_: Lambda.Context, request: Request, callback: @escaping (Result<Response, Error>) -> Void) in
// TODO: something useful
callback(.success(Response(message: "Hello, \(request.name)!")))
}
// set LOCAL_SERVER_ENABLED env variable to "true" to start the local server simulator
// which will allow local debugging
Copy link
Contributor

@ktoso ktoso May 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Ok, so I've internalized the feedback:

this came up when demoing the library to people, the idea of needing to add #if DEBUG on their side or repeatedly remove the withLocalServer was a point of push back

yet still, looking tat this has me worried. It's a bit too magic... some magical variable etc...

So... Is there another way? I think there might be, what about this:

let isDebugMode = debugModeInDebugBuilds && _isDebugAssertConfiguration() // this exists in all Swifts

Gains:

  • no flags to set, just rely on "debug vs release build" semantics
  • noone should ever run debug builds on lambda anyway
  • possible to override if one wants to

WDYT? @tomerd @fabianfett

Copy link
Contributor Author

@tomerd tomerd May 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@ktoso
iiuc the if #DEBUG as implemented in this PR gives you the same results as the isDebugMode line above. the PR also adds an env variable gate as extra caution so if people make a mistake and deploy a debug version to aws its not enabled which would be very bad! it makes things very explicit.

The reason I landed on an env variable gate is that it is something you can turn on/off in Xcode easily - which is the goal of this feature: the ability to debug Lambda locally in the IDE.

cc @fabianfett

Copy link
Contributor Author

@tomerd tomerd May 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

so @fabianfett and myself spoke a but and he pointed out that most iOS devs will not know how to set env variables in Xcode which means this is not a great solution for them. however, we can potentially do something more horrible which is leaning on an env variable Xcode itself sets like XCODE_VERSION_ACTUAL which will make this only work in Xcode unless the developers decides to use this env variables somewhere else which is highly unlikely

meh this won't work - those env variables are not exposed to the targets

Copy link
Contributor Author

@tomerd tomerd May 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

another protection we can do is check for parent pid and only allow this if parent != 1 which normally means debugger, but need to further research if viable

Copy link
Contributor

Choose a reason for hiding this comment

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

Note: Scratch my entire idea about _isDebugAssertConfiguration is devolves into the same as the #if debug stuff.

To be honest not in love with any of the automatic ways... hope you can figure out something that makes folks happy 🤔

Copy link
Contributor Author

@tomerd tomerd May 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

spoke with an Xcode core engineer, and he recommended the env variable way. will double check with a few others too

Lambda.run { (_, request: Request, callback: @escaping (Result<Response, Error>) -> Void) in
// TODO: something useful
callback(.success(Response(message: "Hello, \(request.name)!")))
}
5 changes: 3 additions & 2 deletions Sources/AWSLambdaRuntimeCore/Lambda+LocalServer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,12 @@ extension Lambda {
/// - body: Code to run within the context of the mock server. Typically this would be a Lambda.run function call.
///
/// - note: This API is designed stricly for local testing and is behind a DEBUG flag
public static func withLocalServer(invocationEndpoint: String? = nil, _ body: @escaping () -> Void) throws {
@discardableResult
public static func withLocalServer<Value>(invocationEndpoint: String? = nil, _ body: @escaping () -> Value) throws -> Value {
let server = LocalLambda.Server(invocationEndpoint: invocationEndpoint)
try server.start().wait()
defer { try! server.stop() } // FIXME:
body()
return body()
}
}

Expand Down
65 changes: 42 additions & 23 deletions Sources/AWSLambdaRuntimeCore/Lambda.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,36 +97,55 @@ public enum Lambda {
// for testing and internal use
@discardableResult
internal static func run(configuration: Configuration = .init(), factory: @escaping HandlerFactory) -> Result<Int, Error> {
Backtrace.install()
var logger = Logger(label: "Lambda")
logger.logLevel = configuration.general.logLevel
let _run = { (configuration: Configuration, factory: @escaping HandlerFactory) -> Result<Int, Error> in
Backtrace.install()
var logger = Logger(label: "Lambda")
logger.logLevel = configuration.general.logLevel

var result: Result<Int, Error>!
MultiThreadedEventLoopGroup.withCurrentThreadAsEventLoop { eventLoop in
let lifecycle = Lifecycle(eventLoop: eventLoop, logger: logger, configuration: configuration, factory: factory)
#if DEBUG
let signalSource = trap(signal: configuration.lifecycle.stopSignal) { signal in
logger.info("intercepted signal: \(signal)")
lifecycle.shutdown()
}
#endif

lifecycle.start().flatMap {
lifecycle.shutdownFuture
}.whenComplete { lifecycleResult in
var result: Result<Int, Error>!
MultiThreadedEventLoopGroup.withCurrentThreadAsEventLoop { eventLoop in
let lifecycle = Lifecycle(eventLoop: eventLoop, logger: logger, configuration: configuration, factory: factory)
#if DEBUG
signalSource.cancel()
let signalSource = trap(signal: configuration.lifecycle.stopSignal) { signal in
logger.info("intercepted signal: \(signal)")
lifecycle.shutdown()
}
#endif
eventLoop.shutdownGracefully { error in
if let error = error {
preconditionFailure("Failed to shutdown eventloop: \(error)")

lifecycle.start().flatMap {
lifecycle.shutdownFuture
}.whenComplete { lifecycleResult in
#if DEBUG
signalSource.cancel()
#endif
eventLoop.shutdownGracefully { error in
if let error = error {
preconditionFailure("Failed to shutdown eventloop: \(error)")
}
}
result = lifecycleResult
}
result = lifecycleResult
}

logger.info("shutdown completed")
return result
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

no changes here ^^, just setting a _run local variable with the function body


logger.info("shutdown completed")
return result
// start local server for debugging in DEBUG mode only
#if DEBUG
if Lambda.env("LOCAL_SERVER_ENABLED").flatMap(Bool.init) ?? false {
do {
return try Lambda.withLocalServer {
_run(configuration, factory)
}
} catch {
return .failure(error)
}
} else {
return _run(configuration, factory)
}
#else
return _run(configuration, factory)
#endif
Copy link
Contributor Author

@tomerd tomerd May 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

this is the actual change 👀

hopefully this does not feel too "dangerous" since it behind both #if DEBUG and an env variable. we could potentially add additional protections like a more terribly named env variable, print a warning when running in this mode, or checking for an env variable that should indicate we are running in real lambda

}
}