From d35eb5c8f9a9f74229ec536aa68b52ae9e865f4d Mon Sep 17 00:00:00 2001
From: William Carroll <wpcarro@gmail.com>
Date: Sun, 9 Feb 2020 22:09:21 +0000
Subject: Debug os.Signal handling

Problem:

When SIGINT signals we're sent to the token server, it would shut down without
completing the shutdown procedure. The shutdown procedure would persist the
application state (i.e. access and refresh tokens).

This is problematic for the following sequence of events:
t0. Access and refresh tokens retrieved from kv.json and used as app state.
t1. Tokens are refreshed but not persisted. (I'm still unsure how this
    happens). Remember that this means the previous access and refresh tokens
    from t0 are now invalid.
t2. User sends a SIGINT.
t3. Token server shuts down.
t4. Token server is restarted, kv.json is used as the app state even though its
    tokens are now invalid.
t5. Tokens are attempted to refresh, Monzo API rejects the tokens because
    they're invalid.

Now we need to provide the token server with valid access and refresh tokens
otherwise we will repeat the loop described above. This means going through the
client authorization flow again or copying and pasting the tokens logged from
the token server into kv.json. Either scenario is more manual than I'd prefer.

Solution:

Use a buffered channel to receive the os.Signal. I got this idea after reading
these docs: https://golang.org/pkg/os/signal/#Notify and I debugged this issue
shortly thereafter.

I also rearranged the order of operations in main/0 to ensure that
handleInterrupts/0, which registers the event listeners, occurs before
scheduleTokenRefresh/2 is called. This allows the token server to gracefully
shutdown even if it's in the middle of the scheduleTokenRefresh/2 call.
---
 monzo_ynab/tokens.go | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

(limited to 'monzo_ynab')

diff --git a/monzo_ynab/tokens.go b/monzo_ynab/tokens.go
index 7bebeef11729..b417e49faf44 100644
--- a/monzo_ynab/tokens.go
+++ b/monzo_ynab/tokens.go
@@ -159,7 +159,7 @@ func persistTokens(access string, refresh string) {
 // refresh tokens and shutdown the server.
 func handleInterrupts() {
 	// Gracefully handle interruptions.
-	sigs := make(chan os.Signal)
+	sigs := make(chan os.Signal, 1)
 	done := make(chan bool)
 
 	signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM)
@@ -173,7 +173,7 @@ func handleInterrupts() {
 	}()
 
 	<-done
-	log.Println("Received signal to shutdown. Exiting...")
+	log.Println("Exiting...")
 	os.Exit(0)
 }
 
@@ -203,17 +203,7 @@ func main() {
 		accessToken, refreshToken = tokens.AccessToken, tokens.RefreshToken
 		go persistTokens(accessToken, refreshToken)
 		go scheduleTokenRefresh(tokens.ExpiresIn, refreshToken)
-	} else {
-		// If we have tokens, they may be expiring soon. We don't know because
-		// we aren't storing the expiration timestamp in the state or in the
-		// store. Until we have that information, and to be safe, let's refresh
-		// the tokens.
-		scheduleTokenRefresh(0, refreshToken)
 	}
-
-	// Gracefully shutdown.
-	go handleInterrupts()
-
 	// Manage application state.
 	go func() {
 		state := &state{accessToken, refreshToken}
@@ -232,6 +222,15 @@ func main() {
 		}
 	}()
 
+	// Gracefully shutdown.
+	go handleInterrupts()
+
+	// If we have tokens, they may be expiring soon. We don't know because
+	// we aren't storing the expiration timestamp in the state or in the
+	// store. Until we have that information, and to be safe, let's refresh
+	// the tokens.
+	scheduleTokenRefresh(0, refreshToken)
+
 	// Listen to inbound requests.
 	fmt.Println("Listening on http://localhost:4242 ...")
 	log.Fatal(http.ListenAndServe(":4242",
-- 
cgit 1.4.1