Udpates to docs

UPDATES.adoc

@@ -4,7 +4,7 @@ Run Bonob on your server.
 
 == Updates made to original code
 
-* Proper Token handling after login. Also handling of periodic token refresh.
+* Proper Token handling after login. Also handling of periodic token refresh. Something is still funky here after a day or two...
 * Store Tokens in an SQLite database (in mounted `/config` directory).
 * Added variable `BNB_TOKEN_CLEANUP_INTERVAL` with a default of `60` (minutes) to set how often expired tokens should be cleaned up out of the database.
 * Multi-account logins. Register one Bonob and log in with multiple Navidrome users for easy account switching in the Sonos app.
@@ -27,6 +27,7 @@ Run Bonob on your server.
 Bonob now needs a volume to store the token database. In the example below that directory is `/var/containers/bonob`. Adapt as needed.
 Also the example below uses a `bonob` user on the system with ID `1210` and group `100`. The database directory should be owned by that user.
 
+Also for `BNB_SUBSONIC_URL` you can use the internal or external URL. So instead of `https://music.mydomain.com` you could use `http://192.168.1.100:4533` if your Navidrome runs on a server with IP `192.168.1.100`.
 .Example systemd file (`/usr/lib/systemd/system/bonob.service`)
 [source]
 ----

sonos_service/SONOS_SERVICE.adoc

@@ -5,7 +5,7 @@
 +
 image::images/about.png[]
 
-* Navidrome running and available from the Internet. E.g. via https://music.mydomain.com
+* Navidrome running and available from the server that Bonob is running on. This can be a public URL like https://music.mydomain.com or just a local URL like http://192.168.1.100:4533.
 * Bonob running and available from the Internet. E.g. via https://bonob.mydomain.com
 
 You can use any method to make these URLs available. Cloudflare Tunnels, Pangolin, reverse proxy, etc.
@@ -21,7 +21,7 @@ You can use any method to make these URLs available. Cloudflare Tunnels, Pangoli
 *** Service Name: Navidrome
 *** Service Availability: Global
 *** Checkbox checked
-*** Website/Social Media URLs: https://music.mydomain.com (Some URL - e.g. your Navidrome server)
+*** Website/Social Media URLs: https://music.mydomain.com (Some URL - e.g. your Navidrome server). This has to be a valid URL.
 
 ** Sonos Music API
 *** Integration ID: com.mydomain.music (your domain in reverse)

src/smapi.ts

@@ -250,11 +250,10 @@ class SonosSoap {
     logger.debug("Current tokens: " + JSON.stringify(this.tokenStore.getAll()));
     return this.tokenStore.get(token);
   }
-  associateCredentialsForToken(token: string, fullSmapiToken: SmapiToken, oldToken?:string) {
+  associateCredentialsForToken(token: string, fullSmapiToken: SmapiToken) {
     logger.debug("Adding token: " + token + " " + JSON.stringify(fullSmapiToken));
-    if(oldToken) {
-      this.tokenStore.delete(oldToken);
-    }
+    // Don't immediately delete old token to avoid race conditions
+    // The cleanup process will handle expired tokens later
     this.tokenStore.set(token, fullSmapiToken);
   }
 }
@@ -488,11 +487,9 @@ function bindSmapiSoapServiceToExpress(
   const swapToken = (expiredToken: string | undefined) => (newToken: SmapiToken) => {
     logger.debug("oldToken: " + expiredToken);
     logger.debug("newToken: " + JSON.stringify(newToken));
-    if (expiredToken) {
-      sonosSoap.associateCredentialsForToken(newToken.token, newToken, expiredToken);
-    } else {
-      sonosSoap.associateCredentialsForToken(newToken.token, newToken);
-    }
+    // Always add the new token, but don't immediately delete the old one
+    // to avoid race conditions where Sonos might still be using the old token
+    sonosSoap.associateCredentialsForToken(newToken.token, newToken);
     return TE.right(newToken);
   }
 
@@ -540,8 +537,6 @@ function bindSmapiSoapServiceToExpress(
           throw SMAPI_FAULT_LOGIN_UNAUTHORIZED;
         });
     } else if (isExpiredTokenError(authOrFail)) {
-      // Don't pass old token here to avoid circular reference issues with Jest/SOAP
-      // Old expired tokens will be cleaned up by TTL or manual cleanup later
       logger.info("Token expired, attempting refresh...");
       throw await pipe(
         musicService.refreshToken(authOrFail.expiredToken),
@@ -549,7 +544,7 @@ function bindSmapiSoapServiceToExpress(
           logger.info("Token refresh successful, issuing new SMAPI token");
           return smapiAuthTokens.issue(it.serviceToken);
         }),
-        TE.tap(swapToken(undefined)),
+        TE.tap(swapToken(authOrFail.expiredToken)), // Pass the expired token to ensure it gets deleted
         TE.map((newToken) => ({
           Fault: {
             faultcode: "Client.TokenRefreshRequired",
@@ -615,12 +610,10 @@ function bindSmapiSoapServiceToExpress(
                 throw fault.toSmapiFault();
               })
             );
-            // Don't pass old token here to avoid circular reference issues with Jest/SOAP
-            // Old expired tokens will be cleaned up by TTL or manual cleanup later
             return pipe(
               musicService.refreshToken(serviceToken),
               TE.map((it) => smapiAuthTokens.issue(it.serviceToken)),
-              TE.tap(swapToken(undefined)), // ignores the return value, like a tee or peek
+              TE.tap(swapToken(serviceToken)), // Pass the expired token to ensure it gets deleted
               TE.map((it) => ({
                 refreshAuthTokenResult: {
                   authToken: it.token,

src/smapi_token_store.ts

@@ -45,9 +45,9 @@ export class InMemorySmapiTokenStore implements SmapiTokenStore {
         // Do NOT delete ExpiredTokenError as those can still be refreshed
         if (E.isLeft(verifyResult)) {
           const error = verifyResult.left;
-          // Only delete invalid tokens, not expired ones (which can be refreshed)
-          if (error._tag === 'InvalidTokenError') {
-            logger.debug(`Deleting invalid token from in-memory store`);
+          // Delete both invalid and expired tokens to prevent accumulation
+          if (error._tag === 'InvalidTokenError' || error._tag === 'ExpiredTokenError') {
+            logger.debug(`Deleting ${error._tag} token from in-memory store`);
             delete this.tokens[tokenKey];
             deletedCount++;
           }
@@ -146,9 +146,9 @@ export class FileSmapiTokenStore implements SmapiTokenStore {
         // Do NOT delete ExpiredTokenError as those can still be refreshed
         if (E.isLeft(verifyResult)) {
           const error = verifyResult.left;
-          // Only delete invalid tokens, not expired ones (which can be refreshed)
-          if (error._tag === 'InvalidTokenError') {
-            logger.debug(`Deleting invalid token from file store`);
+          // Delete both invalid and expired tokens to prevent accumulation
+          if (error._tag === 'InvalidTokenError' || error._tag === 'ExpiredTokenError') {
+            logger.debug(`Deleting ${error._tag} token from file store`);
             delete this.tokens[tokenKey];
             deletedCount++;
           }

src/sqlite_smapi_token_store.ts

@@ -126,9 +126,9 @@ export class SQLiteSmapiTokenStore implements SmapiTokenStore {
           // Do NOT delete ExpiredTokenError as those can still be refreshed
           if (E.isLeft(verifyResult)) {
             const error = verifyResult.left;
-            // Only delete invalid tokens, not expired ones (which can be refreshed)
-            if (error._tag === 'InvalidTokenError') {
-              logger.debug(`Deleting invalid token from SQLite store`);
+            // Delete both invalid and expired tokens to prevent accumulation
+            if (error._tag === 'InvalidTokenError' || error._tag === 'ExpiredTokenError') {
+              logger.debug(`Deleting ${error._tag} token from SQLite store`);
               this.delete(tokenKey);
               deletedCount++;
             }