Skip to content

Commit

Permalink
fix(#47): improve error handling and error logging
Browse files Browse the repository at this point in the history
  • Loading branch information
joeyhage committed Aug 8, 2023
1 parent ab6a228 commit 9e50bf3
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 15 deletions.
2 changes: 1 addition & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ module.exports = {
preset: 'ts-jest',
testEnvironment: 'node',
setupFiles: ['dotenv/config'],
testMatch: ['**/__tests__/**/*.[jt]s?(x)', '**/?(*.)+(spec|test|it).[jt]s?(x)'],
testMatch: ['**/__tests__/**/*.ts?(x)', '**/?(*.)+(spec|test|it).ts?(x)'],
};
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"displayName": "Homebridge Spotify Speaker",
"name": "homebridge-spotify-speaker",
"version": "1.3.1",
"version": "1.3.2-beta.1",
"description": "Homebridge plugin that creates a speaker that plays a specific Spotify playlist",
"license": "MIT",
"author": "Joey Hage <[email protected]>",
Expand Down
32 changes: 28 additions & 4 deletions src/spotify-api-wrapper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,14 @@ test('should re-attempt to retrieve device list once', async () => {
expect(mockSpotifyWebApi.getMyDevices).toHaveBeenCalledTimes(2);
});

test('should re-attempt wrapped request once when 404', async () => {
test('should re-attempt wrapped request once on HTTP 404', async () => {
// given
const wrapper = getSpotifyApiWrapper();

const mockSpotifyWebApi = getMockedSpotifyWebApi();
mockSpotifyWebApi.getMyCurrentPlaybackState
.mockRejectedValueOnce(new WebapiError('test', {} as WebapiErrorBody, {}, 404, 'device not found'))
.mockResolvedValueOnce(fakeGetMyCurrentPlaybackState);
.mockResolvedValueOnce(fakeCurrentPlaybackResponse);

// when
const playbackState = await wrapper.getPlaybackState();
Expand All @@ -46,7 +46,7 @@ test('should re-attempt wrapped request once when 404', async () => {
expect(mockSpotifyWebApi.getMyCurrentPlaybackState).toHaveBeenCalledTimes(2);
});

test('should throw SpotifyDeviceNotFoundError after two 404s', async () => {
test('should throw SpotifyDeviceNotFoundError on second HTTP 404', async () => {
// given
const wrapper = getSpotifyApiWrapper();

Expand All @@ -62,6 +62,24 @@ test('should throw SpotifyDeviceNotFoundError after two 404s', async () => {
await expect(playbackState).rejects.toThrow(SpotifyDeviceNotFoundError);
});

test('should refresh token on HTTP 401', async () => {
// given
const wrapper = getSpotifyApiWrapper();

const mockSpotifyWebApi = getMockedSpotifyWebApi();
mockSpotifyWebApi.getMyCurrentPlaybackState
.mockRejectedValueOnce(new WebapiError('test', {} as WebapiErrorBody, {}, 401, 'unauthorized'))
.mockResolvedValueOnce(fakeCurrentPlaybackResponse);
mockSpotifyWebApi.refreshAccessToken.mockResolvedValueOnce(fakeAccessTokenResponse);

// 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,
Expand Down Expand Up @@ -96,8 +114,14 @@ const fakeGetMyDevicesResponse = {
statusCode: 200,
};

const fakeGetMyCurrentPlaybackState = {
const fakeCurrentPlaybackResponse = {
body: { is_playing: false } as SpotifyApi.CurrentPlaybackResponse,
headers: {},
statusCode: 200,
};

const fakeAccessTokenResponse = {
body: { access_token: 'zzz', expires_in: 0, scope: 'fake_scope', token_type: 'fake_type' },
headers: {},
statusCode: 200,
};
30 changes: 25 additions & 5 deletions src/spotify-api-wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,9 @@ export class SpotifyApiWrapper {
return res.body.devices;
} catch (error) {
if (isFirstAttempt) {
return new Promise((resolve) => {
return new Promise((resolve, reject) => {
setTimeout(() => {
this.getMyDevices(false).then(resolve);
this.getMyDevices(false).then(resolve).catch(reject);
}, 500);
});
} else {
Expand Down Expand Up @@ -167,6 +167,21 @@ export class SpotifyApiWrapper {
}

private async wrappedRequest<T>(cb: () => Promise<T>, isFirstAttempt = true): Promise<T | undefined> {
const retry = (): Promise<T | undefined> => {
return new Promise((resolve, reject) => {
setTimeout(() => {
this.wrappedRequest(cb, false).then(resolve).catch(reject);
}, 500);
});
};

const throwSpotifyDeviceNotFound = (errorMessage: string): Promise<T | undefined> => {
return new Promise((_, reject) => {
this.log.error('SpotifyDeviceNotFoundError:', JSON.stringify(errorMessage));
reject(new SpotifyDeviceNotFoundError());
});
};

try {
const response = await cb();
return response;
Expand All @@ -179,10 +194,10 @@ export class SpotifyApiWrapper {

const areTokensRefreshed = await this.refreshTokens();
if (areTokensRefreshed) {
return this.wrappedRequest(cb, false);
return retry();
}
} else if (webApiError.statusCode === 404) {
return isFirstAttempt ? this.wrappedRequest(cb, false) : Promise.reject(new SpotifyDeviceNotFoundError());
return isFirstAttempt ? retry() : throwSpotifyDeviceNotFound(JSON.stringify(errorMessage));
}
errorMessage = webApiError.body;
}
Expand All @@ -192,7 +207,12 @@ export class SpotifyApiWrapper {
}
}

const WebApiErrorTypes = ['WebapiError', 'WebapiRegularError', 'WebapiAuthenticationError', 'WebapiPlayerError'];
// 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';
return (
WebApiErrorTypes.includes(error.constructor.name) ||
WebApiErrorTypes.includes(Object.getPrototypeOf(error.constructor).name) ||
WebApiErrorTypes.includes(Object.getPrototypeOf(error).constructor.name)
);
}
4 changes: 2 additions & 2 deletions src/spotify-speaker-accessory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,9 @@ export class SpotifySpeakerAccessory {
if (match?.id) {
this.device.spotifyDeviceId = match.id;
} else if (isFirstAttempt) {
await new Promise((resolve) => {
await new Promise((resolve, reject) => {
setTimeout(() => {
this.setCurrentStates(false).then(resolve);
this.setCurrentStates(false).then(resolve).catch(reject);
}, 500);
});
} else {
Expand Down

0 comments on commit 9e50bf3

Please sign in to comment.