Skip to content

Commit 78f24ee

Browse files
fallenbagelfallenbagel
committed
fix: handle already-removed media in radarr/sonarr delete
When a movie is absent from the *arr library, removeMovie/removeSeries now returns instead of throwing, so the Seerr record can still be cleaned up. fix #3112
1 parent ebac489 commit 78f24ee

5 files changed

Lines changed: 241 additions & 25 deletions

File tree

‎server/api/servarr/radarr.test.ts‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
import assert from 'node:assert/strict';
2+
import { afterEach, describe, it, mock } from 'node:test';
3+
4+
import type { AxiosInstance } from 'axios';
5+
6+
import RadarrAPI from '@server/api/servarr/radarr';
7+
8+
function buildRadarr(): RadarrAPI {
9+
return new RadarrAPI({ url: 'http://localhost:7878/api/v3', apiKey: 'test' });
10+
}
11+
12+
function getAxios(radarr: RadarrAPI): AxiosInstance {
13+
return (radarr as unknown as { axios: AxiosInstance }).axios;
14+
}
15+
16+
describe('RadarrAPI removeMovie', () => {
17+
afterEach(() => mock.restoreAll());
18+
19+
it('removes the movie when it exists in the library', async () => {
20+
const radarr = buildRadarr();
21+
mock.method(RadarrAPI.prototype, 'getMovieByTmdbId', async () => ({
22+
id: 7,
23+
title: 'Test Movie',
24+
}));
25+
const del = mock.method(getAxios(radarr), 'delete', async () => ({}));
26+
27+
await radarr.removeMovie(550);
28+
29+
assert.strictEqual(del.mock.callCount(), 1);
30+
assert.strictEqual(del.mock.calls[0].arguments[0], '/movie/7');
31+
});
32+
33+
it('does nothing when the movie is not in the library', async () => {
34+
const radarr = buildRadarr();
35+
mock.method(RadarrAPI.prototype, 'getMovieByTmdbId', async () => ({
36+
id: 0,
37+
title: 'Test Movie',
38+
}));
39+
const del = mock.method(getAxios(radarr), 'delete', async () => ({}));
40+
41+
await radarr.removeMovie(550);
42+
43+
assert.strictEqual(del.mock.callCount(), 0);
44+
});
45+
46+
it('ignores a 404 when the movie was already removed in Radarr', async () => {
47+
const radarr = buildRadarr();
48+
mock.method(RadarrAPI.prototype, 'getMovieByTmdbId', async () => ({
49+
id: 7,
50+
title: 'Test Movie',
51+
}));
52+
mock.method(getAxios(radarr), 'delete', async () => {
53+
throw { response: { status: 404 } };
54+
});
55+
56+
await assert.doesNotReject(() => radarr.removeMovie(550));
57+
});
58+
59+
it('rethrows errors other than 404', async () => {
60+
const radarr = buildRadarr();
61+
mock.method(RadarrAPI.prototype, 'getMovieByTmdbId', async () => ({
62+
id: 7,
63+
title: 'Test Movie',
64+
}));
65+
mock.method(getAxios(radarr), 'delete', async () => {
66+
throw { response: { status: 500 } };
67+
});
68+
69+
await assert.rejects(() => radarr.removeMovie(550));
70+
});
71+
});
72+
73+
describe('RadarrAPI getMovieByTmdbId', () => {
74+
afterEach(() => mock.restoreAll());
75+
76+
it('rethrows a 401 from the lookup with the status intact', async () => {
77+
const radarr = buildRadarr();
78+
mock.method(getAxios(radarr), 'get', async () => {
79+
throw { response: { status: 401 } };
80+
});
81+
82+
await assert.rejects(
83+
() => radarr.getMovieByTmdbId(550),
84+
(e: unknown) =>
85+
(e as { response?: { status?: number } }).response?.status === 401
86+
);
87+
});
88+
89+
it('throws "Movie not found" when the lookup returns no results', async () => {
90+
const radarr = buildRadarr();
91+
mock.method(getAxios(radarr), 'get', async () => ({ data: [] }));
92+
93+
await assert.rejects(() => radarr.getMovieByTmdbId(550), {
94+
message: 'Movie not found',
95+
});
96+
});
97+
});

‎server/api/servarr/radarr.ts‎

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import logger from '@server/logger';
2+
import type { AxiosResponse } from 'axios';
23
import ServarrBase from './base';
34

45
export interface RadarrMovieOptions {
@@ -93,26 +94,27 @@ class RadarrAPI extends ServarrBase<{ movieId: number }> {
9394
};
9495

9596
public async getMovieByTmdbId(id: number): Promise<RadarrMovie> {
97+
let response: AxiosResponse<RadarrMovie[]>;
9698
try {
97-
const response = await this.axios.get<RadarrMovie[]>('/movie/lookup', {
99+
response = await this.axios.get<RadarrMovie[]>('/movie/lookup', {
98100
params: {
99101
term: `tmdb:${id}`,
100102
},
101103
});
102-
103-
if (!response.data[0]) {
104-
throw new Error('Movie not found');
105-
}
106-
107-
return response.data[0];
108104
} catch (e) {
109105
logger.error('Error retrieving movie by TMDB ID', {
110106
label: 'Radarr API',
111107
errorMessage: e.message,
112108
tmdbId: id,
113109
});
114-
throw new Error('Movie not found', { cause: e });
110+
throw e;
115111
}
112+
113+
if (!response.data[0]) {
114+
throw new Error('Movie not found');
115+
}
116+
117+
return response.data[0];
116118
}
117119

118120
public addMovie = async (
@@ -270,6 +272,12 @@ class RadarrAPI extends ServarrBase<{ movieId: number }> {
270272
public removeMovie = async (movieId: number): Promise<void> => {
271273
try {
272274
const { id, title } = await this.getMovieByTmdbId(movieId);
275+
if (!id) {
276+
logger.info(`[Radarr] Movie not in library, nothing to remove`, {
277+
tmdbId: movieId,
278+
});
279+
return;
280+
}
273281
await this.axios.delete(`/movie/${id}`, {
274282
params: {
275283
deleteFiles: true,
@@ -278,9 +286,10 @@ class RadarrAPI extends ServarrBase<{ movieId: number }> {
278286
});
279287
logger.info(`[Radarr] Removed movie ${title}`);
280288
} catch (e) {
281-
throw new Error(`[Radarr] Failed to remove movie: ${e.message}`, {
282-
cause: e,
283-
});
289+
if (e?.response?.status === 404) {
290+
return;
291+
}
292+
throw e;
284293
}
285294
};
286295

‎server/api/servarr/sonarr.test.ts‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
import assert from 'node:assert/strict';
2+
import { afterEach, describe, it, mock } from 'node:test';
3+
4+
import type { AxiosInstance } from 'axios';
5+
6+
import SonarrAPI from '@server/api/servarr/sonarr';
7+
8+
function buildSonarr(): SonarrAPI {
9+
return new SonarrAPI({ url: 'http://localhost:8989/api/v3', apiKey: 'test' });
10+
}
11+
12+
function getAxios(sonarr: SonarrAPI): AxiosInstance {
13+
return (sonarr as unknown as { axios: AxiosInstance }).axios;
14+
}
15+
16+
describe('SonarrAPI removeSeries', () => {
17+
afterEach(() => mock.restoreAll());
18+
19+
it('removes the series when it exists in the library', async () => {
20+
const sonarr = buildSonarr();
21+
mock.method(SonarrAPI.prototype, 'getSeriesByTvdbId', async () => ({
22+
id: 9,
23+
title: 'Test Series',
24+
}));
25+
const del = mock.method(getAxios(sonarr), 'delete', async () => ({}));
26+
27+
await sonarr.removeSeries(1234);
28+
29+
assert.strictEqual(del.mock.callCount(), 1);
30+
assert.strictEqual(del.mock.calls[0].arguments[0], '/series/9');
31+
});
32+
33+
it('does nothing when the series is not in the library', async () => {
34+
const sonarr = buildSonarr();
35+
mock.method(SonarrAPI.prototype, 'getSeriesByTvdbId', async () => ({
36+
id: 0,
37+
title: 'Test Series',
38+
}));
39+
const del = mock.method(getAxios(sonarr), 'delete', async () => ({}));
40+
41+
await sonarr.removeSeries(1234);
42+
43+
assert.strictEqual(del.mock.callCount(), 0);
44+
});
45+
46+
it('ignores a 404 when the series was already removed in Sonarr', async () => {
47+
const sonarr = buildSonarr();
48+
mock.method(SonarrAPI.prototype, 'getSeriesByTvdbId', async () => ({
49+
id: 9,
50+
title: 'Test Series',
51+
}));
52+
mock.method(getAxios(sonarr), 'delete', async () => {
53+
throw { response: { status: 404 } };
54+
});
55+
56+
await assert.doesNotReject(() => sonarr.removeSeries(1234));
57+
});
58+
59+
it('rethrows errors other than 404', async () => {
60+
const sonarr = buildSonarr();
61+
mock.method(SonarrAPI.prototype, 'getSeriesByTvdbId', async () => ({
62+
id: 9,
63+
title: 'Test Series',
64+
}));
65+
mock.method(getAxios(sonarr), 'delete', async () => {
66+
throw { response: { status: 500 } };
67+
});
68+
69+
await assert.rejects(() => sonarr.removeSeries(1234));
70+
});
71+
});
72+
73+
describe('SonarrAPI getSeriesByTvdbId', () => {
74+
afterEach(() => mock.restoreAll());
75+
76+
it('rethrows a 401 from the lookup with the status intact', async () => {
77+
const sonarr = buildSonarr();
78+
mock.method(getAxios(sonarr), 'get', async () => {
79+
throw { response: { status: 401 } };
80+
});
81+
82+
await assert.rejects(
83+
() => sonarr.getSeriesByTvdbId(1234),
84+
(e: unknown) =>
85+
(e as { response?: { status?: number } }).response?.status === 401
86+
);
87+
});
88+
89+
it('throws "Series not found" when the lookup returns no results', async () => {
90+
const sonarr = buildSonarr();
91+
mock.method(getAxios(sonarr), 'get', async () => ({ data: [] }));
92+
93+
await assert.rejects(() => sonarr.getSeriesByTvdbId(1234), {
94+
message: 'Series not found',
95+
});
96+
});
97+
});

‎server/api/servarr/sonarr.ts‎

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import logger from '@server/logger';
2+
import type { AxiosResponse } from 'axios';
23
import ServarrBase from './base';
34

45
export interface SonarrSeason {
@@ -166,26 +167,27 @@ class SonarrAPI extends ServarrBase<{
166167
}
167168

168169
public async getSeriesByTvdbId(id: number): Promise<SonarrSeries> {
170+
let response: AxiosResponse<SonarrSeries[]>;
169171
try {
170-
const response = await this.axios.get<SonarrSeries[]>('/series/lookup', {
172+
response = await this.axios.get<SonarrSeries[]>('/series/lookup', {
171173
params: {
172174
term: `tvdb:${id}`,
173175
},
174176
});
175-
176-
if (!response.data[0]) {
177-
throw new Error('Series not found');
178-
}
179-
180-
return response.data[0];
181177
} catch (e) {
182178
logger.error('Error retrieving series by tvdb ID', {
183179
label: 'Sonarr API',
184180
errorMessage: e.message,
185181
tvdbId: id,
186182
});
187-
throw new Error('Series not found', { cause: e });
183+
throw e;
184+
}
185+
186+
if (!response.data[0]) {
187+
throw new Error('Series not found');
188188
}
189+
190+
return response.data[0];
189191
}
190192

191193
public async addSeries(options: AddSeriesOptions): Promise<SonarrSeries> {
@@ -413,6 +415,12 @@ class SonarrAPI extends ServarrBase<{
413415
public removeSeries = async (serieId: number): Promise<void> => {
414416
try {
415417
const { id, title } = await this.getSeriesByTvdbId(serieId);
418+
if (!id) {
419+
logger.info(`[Sonarr] Series not in library, nothing to remove`, {
420+
tvdbId: serieId,
421+
});
422+
return;
423+
}
416424
await this.axios.delete(`/series/${id}`, {
417425
params: {
418426
deleteFiles: true,
@@ -421,9 +429,10 @@ class SonarrAPI extends ServarrBase<{
421429
});
422430
logger.info(`[Sonarr] Removed series ${title}`);
423431
} catch (e) {
424-
throw new Error(`[Sonarr] Failed to remove series: ${e.message}`, {
425-
cause: e,
426-
});
432+
if (e?.response?.status === 404) {
433+
return;
434+
}
435+
throw e;
427436
}
428437
};
429438

‎server/routes/media.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ mediaRoutes.delete(
254254
mediaId: media.id,
255255
}
256256
);
257-
return;
257+
return res.status(204).send();
258258
}
259259

260260
let service;
@@ -284,11 +284,15 @@ mediaRoutes.delete(
284284

285285
return res.status(204).send();
286286
} catch (e) {
287-
logger.error('Something went wrong fetching media in delete request', {
287+
if (e instanceof EntityNotFoundError) {
288+
return next({ status: 404, message: 'Media not found' });
289+
}
290+
logger.error('Something went wrong deleting media file', {
288291
label: 'Media',
292+
mediaId: req.params.id,
289293
message: e.message,
290294
});
291-
next({ status: 404, message: 'Media not found' });
295+
next({ status: 500, message: 'Failed to delete media file' });
292296
}
293297
}
294298
);

0 commit comments

Comments
 (0)