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

tomerd · 2020-05-27T04:41:28Z

motivation: make using the local debugging server easier to turn off/on without the need to change code when preparing to deploy

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

changes:

add code to lambda so that in debug mode only, if the LOCAL_SERVER_ENABLED env variable is set the local degging server is started
update example code

…iable motivation: make using the local debugging server easier to turn off/on without the need to change code when oyu are preparing to deploy changes: * add code to lambda so that in debug mode only, if the LOCAL_SERVER_ENABLED env variable is set the local degging server is started * update example code

tomerd · 2020-05-27T04:42:34Z

Sources/AWSLambdaRuntimeCore/Lambda.swift

            }
+
+            logger.info("shutdown completed")
+            return result
        }


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

tomerd · 2020-05-27T04:44:19Z

Sources/AWSLambdaRuntimeCore/Lambda.swift

+        }
+        #else
+        return _run(configuration, factory)
+        #endif


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

fabianfett

Does this have to be a library feature?

I used the following approach in my test mains:

#if DEBUG
LocalServer.withLambda {
  Lambda.run(Handler.init)
}
#else 
Lambda.run(Handler.init)
#endif

Sure we could save six lines of code in the developers space, but we need to educate about an env variable. I can totally see the need for this, I just wonder if an env variable is the right approach to this.

ktoso · 2020-05-27T07:55:27Z

Examples/LocalDebugging/MyLambda/Sources/MyLambda/main.swift

-        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


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~~

@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

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

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

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 🤔

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

tomerd · 2020-05-27T21:28:27Z

Does this have to be a library feature?

@fabianfett the #if DEBUG approach is the one I used in demos but it got a bit of a negative feedback, hence this PR

eneko · 2020-05-30T15:51:55Z

How about this approach, embedding #if DEBUG inside the withLocalServer closure?

extension Lambda {
    /// Execute code in the context of a mock Lambda server.
    ///
    /// - parameters:
    ///     - invocationEndpoint: The endpoint  to post payloads to.
    ///     - 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 strictly for local testing and is behind a DEBUG flag
    public static func withLocalServer(invocationEndpoint: String? = nil, _ body: @escaping () -> Void) throws {
        #if DEBUG
        let server = LocalLambda.Server(invocationEndpoint: invocationEndpoint)
        try server.start().wait()
        defer { try! server.stop() } // FIXME:
        #endif
        body()
    }
}

Usage

This would start a local server in Debug, and run normally in Release mode:

try Lambda.withLocalServer {
    Lambda.run { (_, request: Request, callback) in
        callback(.success(Response(message: "Hello, \(request.name)!")))
    }
}

chrisaljoudi · 2020-06-01T05:59:35Z

So, uh, I might just be missing a ton of context here (probably am)... but wouldn’t doing this with an env-variable (vs. a build-time-deterministic flag) result in larger binaries?

It’s probably only a slight increase (if at all, perhaps I’m misunderstanding whether the ~~compiler~~ toolchain would link the entire library either way... though it seems like it wouldn’t with WMO on).

Still, the idea of the resulting binary having significantly different behavior based on the presence of an env variable does make me a little nervous at first glance.

It might very well be the right answer—just sharing some thoughts.

Edit: never mind, I just noticed that the Lambda.env check is inside an if #DEBUG anyway, so Release builds would omit it anyways. Sorry.

tomerd · 2020-06-01T17:04:24Z

This would start a local server in Debug, and run normally in Release mode:

@eneko this is def one of the options we considered. the challenge with this approach tho is that the code does not do what it says it does which could lead to further confusion

tomerd · 2020-06-01T17:07:04Z

I just noticed that the Lambda.env check is inside an if #DEBUG anyway, so Release builds would omit it anyways.

exactly - this entire functionality is stripped away when building a release version, which is what developers should be using to deploy to AWS.

the risk if only replying on debug vs. release is that we are all humans so may make a mistake and ship a debug version so the env variable is extra protection

tomerd requested a review from fabianfett May 27, 2020 04:41

tomerd commented May 27, 2020

View reviewed changes

tomerd requested review from ktoso and yim-lee May 27, 2020 05:01

fabianfett reviewed May 27, 2020

View reviewed changes

ktoso reviewed May 27, 2020

View reviewed changes

Merge branch 'master' into feature/easier-local-debugging

c761c44

Merge branch 'master' into feature/easier-local-debugging

83861da

Merge branch 'master' into feature/easier-local-debugging

f3dc137

fabianfett approved these changes Jun 1, 2020

View reviewed changes

fixup

4115008

fabianfett approved these changes Jun 1, 2020

View reviewed changes

fabianfett added this to the 0.1.1 milestone Jun 1, 2020

tomerd merged commit e2ac820 into master Jun 1, 2020

tomerd deleted the feature/easier-local-debugging branch June 1, 2020 19:38

tomerd mentioned this pull request Jun 2, 2020

improve local debugging #109

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

tomerd commented May 27, 2020 •

edited

Loading

tomerd May 27, 2020

tomerd May 27, 2020 •

edited

Loading

fabianfett left a comment

ktoso May 27, 2020 •

edited

Loading

tomerd May 27, 2020 •

edited

Loading

tomerd May 27, 2020 •

edited

Loading

tomerd May 27, 2020 •

edited

Loading

ktoso May 28, 2020

tomerd May 28, 2020 •

edited

Loading

tomerd commented May 27, 2020

eneko commented May 30, 2020

chrisaljoudi commented Jun 1, 2020 •

edited

Loading

tomerd commented Jun 1, 2020

tomerd commented Jun 1, 2020 •

edited

Loading

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

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

Conversation

tomerd commented May 27, 2020 • edited Loading

tomerd May 27, 2020

Choose a reason for hiding this comment

tomerd May 27, 2020 • edited Loading

Choose a reason for hiding this comment

fabianfett left a comment

Choose a reason for hiding this comment

ktoso May 27, 2020 • edited Loading

Choose a reason for hiding this comment

tomerd May 27, 2020 • edited Loading

Choose a reason for hiding this comment

tomerd May 27, 2020 • edited Loading

Choose a reason for hiding this comment

tomerd May 27, 2020 • edited Loading

Choose a reason for hiding this comment

ktoso May 28, 2020

Choose a reason for hiding this comment

tomerd May 28, 2020 • edited Loading

Choose a reason for hiding this comment

tomerd commented May 27, 2020

eneko commented May 30, 2020

Usage

chrisaljoudi commented Jun 1, 2020 • edited Loading

tomerd commented Jun 1, 2020

tomerd commented Jun 1, 2020 • edited Loading

tomerd commented May 27, 2020 •

edited

Loading

tomerd May 27, 2020 •

edited

Loading

ktoso May 27, 2020 •

edited

Loading

tomerd May 27, 2020 •

edited

Loading

tomerd May 27, 2020 •

edited

Loading

tomerd May 27, 2020 •

edited

Loading

tomerd May 28, 2020 •

edited

Loading

chrisaljoudi commented Jun 1, 2020 •

edited

Loading

tomerd commented Jun 1, 2020 •

edited

Loading