fix(#47): add retries to api calls to mitigate sleeping Wi-Fi speakers

Author: joeyhage

.github/workflows/build.yml

@@ -39,3 +39,6 @@ jobs:
         run: npm run build
         env:
           CI: true
+
+      - name: Test the project
+        run: npm run test-ci

jest.config.js

@@ -3,4 +3,5 @@ module.exports = {
   preset: 'ts-jest',
   testEnvironment: 'node',
   setupFiles: ['dotenv/config'],
+  testMatch: [ '**/__tests__/**/*.[jt]s?(x)', '**/?(*.)+(spec|test|it).[jt]s?(x)' ],
 };

package.json

@@ -29,7 +29,8 @@
     "prepare": "husky install",
     "prepublishOnly": "npm run lint && npm run format && npm run build",
     "watch": "npm run build && npm link && nodemon",
-    "test": "jest --silent=false"
+    "test": "jest --silent=false --runInBand",
+    "test-ci": "jest --silent=true --runInBand --testPathPattern='test.ts'"
   },
   "keywords": [
     "homebridge-plugin",

src/spotify-api-wrapper.spec.ts src/spotify-api-wrapper.it.ts

@@ -3,15 +3,7 @@ import { SpotifyApiWrapper } from './spotify-api-wrapper';
 
 it('should authenticate and persist tokens', async () => {
   // given
-  const wrapper = new SpotifyApiWrapper(
-    console as Logger,
-    {
-      spotifyAuthCode: process.env.SPOTIFY_AUTH_CODE!,
-      spotifyClientId: process.env.SPOTIFY_CLIENT_ID!,
-      spotifyClientSecret: process.env.SPOTIFY_CLIENT_SECRET!,
-    } as unknown as PlatformConfig,
-    { user: { persistPath: () => '.' } } as API,
-  );
+  const wrapper = getSpotifyApiWrapper();
 
   // when
   const result = await wrapper.authenticate();
@@ -23,15 +15,7 @@ it('should authenticate and persist tokens', async () => {
 
 it('should retrieve device list', async () => {
   // given
-  const wrapper = new SpotifyApiWrapper(
-    console as Logger,
-    {
-      spotifyAuthCode: process.env.SPOTIFY_AUTH_CODE!,
-      spotifyClientId: process.env.SPOTIFY_CLIENT_ID!,
-      spotifyClientSecret: process.env.SPOTIFY_CLIENT_SECRET!,
-    } as unknown as PlatformConfig,
-    { user: { persistPath: () => '.' } } as API,
-  );
+  const wrapper = getSpotifyApiWrapper();
 
   // when
   await wrapper.authenticate();
@@ -43,7 +27,18 @@ it('should retrieve device list', async () => {
 
 it('should retrieve playback state', async () => {
   // given
-  const wrapper = new SpotifyApiWrapper(
+  const wrapper = getSpotifyApiWrapper();
+
+  // when
+  await wrapper.authenticate();
+  const state = await wrapper.getPlaybackState();
+
+  // then
+  expect(state?.statusCode && state?.statusCode >= 200 && state?.statusCode <= 300).toBeTruthy();
+});
+
+function getSpotifyApiWrapper(): SpotifyApiWrapper {
+  return new SpotifyApiWrapper(
     console as Logger,
     {
       spotifyAuthCode: process.env.SPOTIFY_AUTH_CODE!,
@@ -52,11 +47,4 @@ it('should retrieve playback state', async () => {
     } as unknown as PlatformConfig,
     { user: { persistPath: () => '.' } } as API,
   );
-
-  // when
-  await wrapper.authenticate();
-  const state = await wrapper.getPlaybackState();
-
-  // then
-  expect(state?.statusCode).toEqual(200);
-});
+}

src/spotify-api-wrapper.test.ts

@@ -0,0 +1,86 @@
+import { API, Logger, PlatformConfig } from 'homebridge';
+import SpotifyWebApi from 'spotify-web-api-node';
+import { SpotifyApiWrapper } from './spotify-api-wrapper';
+import type { WebapiErrorBody } from './types';
+import { WebapiError } from './types';
+
+jest.mock('spotify-web-api-node');
+const spotifyWebApiMocks = SpotifyWebApi as jest.MockedClass<typeof SpotifyWebApi>;
+
+beforeEach(() => {
+  spotifyWebApiMocks.mockClear();
+});
+
+test('should re-attempt to retrieve device list once', async () => {
+  // given
+  const wrapper = getSpotifyApiWrapper();
+
+  const mockSpotifyWebApi = getMockedSpotifyWebApi();
+  mockSpotifyWebApi.getMyDevices
+    .mockRejectedValueOnce(new Error('something went wrong'))
+    .mockResolvedValueOnce(fakeGetMyDevicesResponse);
+
+  // when
+  const devices = await wrapper.getMyDevices();
+
+  // then
+  expect(devices?.length).toBe(1);
+  expect(mockSpotifyWebApi.getMyDevices).toHaveBeenCalledTimes(2);
+});
+
+test('should re-attempt wrapped request once when 404', async () => {
+  // given
+  const wrapper = getSpotifyApiWrapper();
+
+  const mockSpotifyWebApi = getMockedSpotifyWebApi();
+  mockSpotifyWebApi.getMyCurrentPlaybackState
+    .mockRejectedValueOnce(new WebapiError('test', {} as WebapiErrorBody, {}, 404, 'device not found'))
+    .mockResolvedValueOnce(fakeGetMyCurrentPlaybackState);
+
+  // when
+  const playbackState = await wrapper.getPlaybackState();
+
+  // then
+  expect(playbackState?.body.is_playing).toBe(false);
+  expect(mockSpotifyWebApi.getMyCurrentPlaybackState).toHaveBeenCalledTimes(2);
+});
+
+function getSpotifyApiWrapper(): SpotifyApiWrapper {
+  return new SpotifyApiWrapper(
+    console as Logger,
+    {
+      spotifyAuthCode: '',
+      spotifyClientId: '',
+      spotifyClientSecret: '',
+    } as unknown as PlatformConfig,
+    { user: { persistPath: () => '.' } } as API,
+  );
+}
+
+function getMockedSpotifyWebApi(): jest.Mocked<SpotifyWebApi> {
+  return spotifyWebApiMocks.mock.instances[0] as jest.Mocked<SpotifyWebApi>;
+}
+
+const fakeGetMyDevicesResponse = {
+  body: {
+    devices: [
+      {
+        id: 'id',
+        is_active: false,
+        is_restricted: false,
+        is_private_session: false,
+        name: 'test',
+        type: 'type',
+        volume_percent: 0,
+      },
+    ],
+  },
+  headers: {},
+  statusCode: 200,
+};
+
+const fakeGetMyCurrentPlaybackState = {
+  body: { is_playing: false } as SpotifyApi.CurrentPlaybackResponse,
+  headers: {},
+  statusCode: 200,
+};

src/spotify-api-wrapper.ts

@@ -4,7 +4,7 @@ import { SpotifyDeviceNotFoundError } from './errors';
 
 import type { API, Logger, PlatformConfig } from 'homebridge';
 import type { HomebridgeSpotifySpeakerDevice } from './spotify-speaker-accessory';
-import type { SpotifyPlaybackState, WebapiError } from './types';
+import { SpotifyPlaybackState, WebapiError } from './types';
 
 const DEFAULT_SPOTIFY_CALLBACK = 'https://example.com/callback';
 
@@ -95,13 +95,21 @@ export class SpotifyApiWrapper {
     await this.wrappedRequest(() => this.spotifyApi.setVolume(volume, { device_id: deviceId }));
   }
 
-  async getMyDevices() {
+  async getMyDevices(isFirstAttempt = true): Promise<SpotifyApi.UserDevice[]> {
     try {
       const res = await this.spotifyApi.getMyDevices();
       return res.body.devices;
     } catch (error) {
-      this.log.error('Failed to fetch available Spotify devices.');
-      return null;
+      if (isFirstAttempt) {
+        return new Promise((resolve) => {
+          setTimeout(() => {
+            this.getMyDevices(false).then(resolve);
+          }, 500);
+        });
+      } else {
+        this.log.error('Failed to fetch available Spotify devices.', error);
+        return [];
+      }
     }
   }
 
@@ -158,29 +166,33 @@ export class SpotifyApiWrapper {
     return true;
   }
 
-  private async wrappedRequest<T>(cb: () => Promise<T>): Promise<T | undefined> {
+  private async wrappedRequest<T>(cb: () => Promise<T>, isFirstAttempt = true): Promise<T | undefined> {
     try {
       const response = await cb();
       return response;
     } catch (error: unknown) {
-      // eslint-disable-next-line @typescript-eslint/no-explicit-any
-      const isWebApiError = Object.getPrototypeOf((error as any).constructor).name === 'WebapiError';
-
-      if (isWebApiError) {
-        if ((error as WebapiError).statusCode === 401) {
+      let errorMessage = error;
+      if (isWebApiError(error) && isFirstAttempt) {
+        const webApiError = error as WebapiError;
+        if (webApiError.statusCode === 401) {
           this.log.debug('Access token has expired, attempting token refresh');
 
           const areTokensRefreshed = await this.refreshTokens();
           if (areTokensRefreshed) {
-            return this.wrappedRequest(cb);
+            return this.wrappedRequest(cb, false);
           }
-        } else if ((error as WebapiError).statusCode === 404) {
-          throw new SpotifyDeviceNotFoundError();
+        } else if (webApiError.statusCode === 404) {
+          return this.wrappedRequest(cb, false);
         }
+        errorMessage = webApiError.body;
       }
 
-      const errorMessage = isWebApiError ? (error as WebapiError).body : error;
       this.log.error('Unexpected error when making a request to Spotify:', JSON.stringify(errorMessage));
     }
   }
 }
+
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+function isWebApiError(error: any): boolean {
+  return error.constructor.name === 'WebapiError' || Object.getPrototypeOf(error.constructor).name === 'WebapiError';
+}

src/spotify-speaker-accessory.ts

@@ -13,7 +13,7 @@ export interface HomebridgeSpotifySpeakerDevice {
 }
 
 export class SpotifySpeakerAccessory {
-  private static DEFAULT_POLL_INTERVAL_MS = 20;
+  private static DEFAULT_POLL_INTERVAL_S = 20;
   private service: Service;
   private activeState: boolean;
   private currentVolume: number;
@@ -59,7 +59,7 @@ export class SpotifySpeakerAccessory {
       if (oldVolume !== this.currentVolume) {
         this.service.updateCharacteristic(this.platform.Characteristic.Brightness, this.currentVolume);
       }
-    }, (this.platform.config.spotifyPollInterval || SpotifySpeakerAccessory.DEFAULT_POLL_INTERVAL_MS) * 1000);
+    }, (this.platform.config.spotifyPollInterval || SpotifySpeakerAccessory.DEFAULT_POLL_INTERVAL_S) * 1000);
   }
 
   handleOnGet(): boolean {
@@ -119,12 +119,18 @@ export class SpotifySpeakerAccessory {
     this.service.getCharacteristic(this.platform.Characteristic.Brightness).updateValue(this.currentVolume);
   }
 
-  private async setCurrentStates() {
+  private async setCurrentStates(isFirstAttempt = true) {
     if (this.device.spotifyDeviceName) {
       const devices = await this.platform.spotifyApiWrapper.getMyDevices();
       const match = devices?.find((device) => device.name === this.device.spotifyDeviceName);
       if (match?.id) {
         this.device.spotifyDeviceId = match.id;
+      } else if (isFirstAttempt) {
+        await new Promise((resolve) => {
+          setTimeout(() => {
+            this.setCurrentStates(false).then(resolve);
+          }, 500);
+        });
       } else {
         this.log.error(
           `spotifyDeviceName '${this.device.spotifyDeviceName}' did not match any Spotify devices. spotifyDeviceName is case sensitive.`,

src/types.ts

@@ -1,4 +1,4 @@
-interface WebapiErrorBody {
+export interface WebapiErrorBody {
   error: {
     message: string;
     status: number;
@@ -8,12 +8,14 @@ interface WebapiErrorBody {
 interface WebapiErrorHeaders {
   [key: string]: string | boolean;
 }
-export interface WebapiError {
-  name: string;
-  body: WebapiErrorBody;
-  headers: WebapiErrorHeaders;
-  statusCode: number;
-  message: string;
+export class WebapiError {
+  constructor(
+    public readonly name: string,
+    public readonly body: WebapiErrorBody,
+    public readonly headers: WebapiErrorHeaders,
+    public readonly statusCode: number,
+    public readonly message: string,
+  ) {}
 }
 
 /**