Skip to content

Commit e1b0fbc

Browse files
jvgomgclaude
andcommitted
fix: bounded retries for Subsonic connection failures (TASK-170)
When a Subsonic server is unreachable (DNS failure, connection refused, timeout), the adapter now retries 3 times with exponential backoff (1s, 2s, 4s) then throws a SubsonicConnectionError with a clear message including the URL and Docker-specific diagnostic hints. Non-connection errors (HTTP 401/403, bad requests) fail immediately without retrying. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 2e7ba81 commit e1b0fbc

5 files changed

Lines changed: 317 additions & 4 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@podkit/core": patch
3+
---
4+
5+
Fix Subsonic connection failures hanging indefinitely instead of failing with a clear error message

packages/podkit-core/src/adapters/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,6 @@ export { DirectoryAdapter, createDirectoryAdapter } from './directory.js';
2121
export type { DirectoryAdapterConfig, ScanProgress } from './directory.js';
2222

2323
// Subsonic adapter
24-
export { SubsonicAdapter, createSubsonicAdapter } from './subsonic.js';
24+
export { SubsonicAdapter, createSubsonicAdapter, SubsonicConnectionError } from './subsonic.js';
2525

2626
export type { SubsonicAdapterConfig } from './subsonic.js';

packages/podkit-core/src/adapters/subsonic.test.ts

Lines changed: 177 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
* Tests use manual mocking of the SubsonicAPI to avoid real network calls.
55
*/
66

7-
import { describe, it, expect } from 'bun:test';
8-
import { SubsonicAdapter } from './subsonic.js';
7+
import { describe, it, expect, beforeEach, afterEach } from 'bun:test';
8+
import { SubsonicAdapter, SubsonicConnectionError } from './subsonic.js';
99
import type { SubsonicAdapterConfig } from './subsonic.js';
1010
import type { Child, AlbumWithSongsID3 } from 'subsonic-api';
1111
import { replayGainToSoundcheck } from '../sync/soundcheck.js';
@@ -522,3 +522,178 @@ describe('SubsonicAdapter artwork presence detection', () => {
522522
expect(t2.hasArtwork).toBe(false);
523523
});
524524
});
525+
526+
// =============================================================================
527+
// Connection Retry Tests
528+
// =============================================================================
529+
530+
describe('SubsonicAdapter connection retries', () => {
531+
const originalFetch = globalThis.fetch;
532+
533+
afterEach(() => {
534+
globalThis.fetch = originalFetch;
535+
});
536+
537+
/**
538+
* Create an adapter with a mocked globalThis.fetch.
539+
* The mock must be installed BEFORE creating the adapter because
540+
* createRetryFetch captures globalThis.fetch at construction time.
541+
*/
542+
function createAdapterWithMockedFetch(mockFetch: typeof globalThis.fetch) {
543+
globalThis.fetch = mockFetch;
544+
return new SubsonicAdapter({
545+
url: 'https://music.example.com',
546+
username: 'testuser',
547+
password: 'testpass',
548+
});
549+
}
550+
551+
it('retries connection errors up to 3 times then throws SubsonicConnectionError', async () => {
552+
let fetchCount = 0;
553+
const adapter = createAdapterWithMockedFetch(async () => {
554+
fetchCount++;
555+
throw new TypeError('fetch failed');
556+
});
557+
558+
const error = await adapter.connect().catch((e) => e);
559+
expect(error).toBeInstanceOf(SubsonicConnectionError);
560+
expect(fetchCount).toBe(3);
561+
});
562+
563+
it('error message includes the server URL', async () => {
564+
const adapter = createAdapterWithMockedFetch(async () => {
565+
throw new TypeError('fetch failed');
566+
});
567+
568+
const error = await adapter.connect().catch((e) => e);
569+
expect(error.message).toContain('https://music.example.com');
570+
});
571+
572+
it('error message includes retry count and diagnostic hints', async () => {
573+
const adapter = createAdapterWithMockedFetch(async () => {
574+
throw new TypeError('fetch failed');
575+
});
576+
577+
const error = await adapter.connect().catch((e) => e);
578+
expect(error.message).toContain('after 3 attempts');
579+
expect(error.message).toContain('Check that the server is running');
580+
expect(error.message).toContain('Docker');
581+
});
582+
583+
it('SubsonicConnectionError has url property', async () => {
584+
const adapter = createAdapterWithMockedFetch(async () => {
585+
throw new TypeError('fetch failed');
586+
});
587+
588+
const error = await adapter.connect().catch((e) => e);
589+
expect(error).toBeInstanceOf(SubsonicConnectionError);
590+
expect(error.url).toBe('https://music.example.com');
591+
});
592+
593+
it('retries on DNS resolution failure (ENOTFOUND)', async () => {
594+
let fetchCount = 0;
595+
const adapter = createAdapterWithMockedFetch(async () => {
596+
fetchCount++;
597+
const err = new Error('getaddrinfo ENOTFOUND music.example.com');
598+
throw err;
599+
});
600+
601+
await expect(adapter.connect()).rejects.toBeInstanceOf(SubsonicConnectionError);
602+
expect(fetchCount).toBe(3);
603+
});
604+
605+
it('retries on connection refused (ECONNREFUSED)', async () => {
606+
let fetchCount = 0;
607+
const adapter = createAdapterWithMockedFetch(async () => {
608+
fetchCount++;
609+
throw new Error('connect ECONNREFUSED 192.168.1.100:4533');
610+
});
611+
612+
await expect(adapter.connect()).rejects.toBeInstanceOf(SubsonicConnectionError);
613+
expect(fetchCount).toBe(3);
614+
});
615+
616+
it('retries on timeout (ETIMEDOUT)', async () => {
617+
let fetchCount = 0;
618+
const adapter = createAdapterWithMockedFetch(async () => {
619+
fetchCount++;
620+
throw new Error('connect ETIMEDOUT 10.0.0.1:443');
621+
});
622+
623+
await expect(adapter.connect()).rejects.toBeInstanceOf(SubsonicConnectionError);
624+
expect(fetchCount).toBe(3);
625+
});
626+
627+
it('succeeds on retry after transient connection failure', async () => {
628+
let fetchCount = 0;
629+
const adapter = createAdapterWithMockedFetch(async () => {
630+
fetchCount++;
631+
if (fetchCount < 3) {
632+
throw new TypeError('fetch failed');
633+
}
634+
// Return a successful Subsonic ping response
635+
return new Response(
636+
JSON.stringify({
637+
'subsonic-response': { status: 'ok', version: '1.16.1' },
638+
}),
639+
{ status: 200, headers: { 'content-type': 'application/json' } }
640+
);
641+
});
642+
643+
// Should not throw — succeeds on 3rd attempt
644+
await adapter.connect();
645+
expect(fetchCount).toBe(3);
646+
});
647+
648+
it('does not retry on non-connection errors', async () => {
649+
let fetchCount = 0;
650+
const adapter = createAdapterWithMockedFetch(async () => {
651+
fetchCount++;
652+
// A non-connection error (e.g., thrown by middleware)
653+
throw new Error('some other error');
654+
});
655+
656+
// Should fail immediately without retrying
657+
// The error wrapping in connect() catches it as a generic connection failure
658+
await expect(adapter.connect()).rejects.toThrow(/Failed to connect/);
659+
expect(fetchCount).toBe(1);
660+
});
661+
662+
it('does not retry when server returns HTTP 401 (authentication failure)', async () => {
663+
let fetchCount = 0;
664+
const adapter = createAdapterWithMockedFetch(async () => {
665+
fetchCount++;
666+
// HTTP 401 is returned as a Response, not thrown — fetch() resolves for HTTP errors.
667+
// The subsonic-api library parses the response and may throw its own error,
668+
// but the fetch layer itself succeeds. We verify fetch is called only once.
669+
return new Response(
670+
JSON.stringify({
671+
'subsonic-response': {
672+
status: 'failed',
673+
error: { code: 40, message: 'Wrong username or password' },
674+
},
675+
}),
676+
{ status: 200, headers: { 'content-type': 'application/json' } }
677+
);
678+
});
679+
680+
// The adapter wraps the "status: failed" response into an error
681+
await expect(adapter.connect()).rejects.toThrow(/Failed to connect/);
682+
// fetch was called exactly once — no retries for auth failures
683+
expect(fetchCount).toBe(1);
684+
});
685+
686+
it('does not retry when server returns HTTP 403 (forbidden)', async () => {
687+
let fetchCount = 0;
688+
const adapter = createAdapterWithMockedFetch(async () => {
689+
fetchCount++;
690+
return new Response('Forbidden', {
691+
status: 403,
692+
headers: { 'content-type': 'text/plain' },
693+
});
694+
});
695+
696+
await expect(adapter.connect()).rejects.toThrow(/Failed to connect/);
697+
expect(fetchCount).toBe(1);
698+
});
699+
});

packages/podkit-core/src/adapters/subsonic.ts

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,130 @@ export interface SubsonicAdapterConfig {
3737
*/
3838
const MIN_ARTWORK_BYTES = 100;
3939

40+
/**
41+
* Maximum number of retry attempts for connection-level failures
42+
* (DNS resolution, connection refused, timeout).
43+
*/
44+
const MAX_RETRIES = 3;
45+
46+
/**
47+
* Timeout per request in milliseconds (30 seconds).
48+
* Prevents indefinite hangs on unresponsive servers.
49+
*/
50+
const REQUEST_TIMEOUT_MS = 30_000;
51+
52+
/**
53+
* Base delay for exponential backoff between retries (in milliseconds).
54+
* Retry 1: 1s, Retry 2: 2s, Retry 3: 4s
55+
*/
56+
const RETRY_BASE_DELAY_MS = 1_000;
57+
58+
/**
59+
* Check if an error is a connection-level failure that should be retried.
60+
*
61+
* Connection errors from fetch() are thrown as TypeError (per the Fetch spec).
62+
* This includes DNS resolution failures, connection refused, network unreachable,
63+
* and AbortError from our timeout signal. HTTP errors (4xx, 5xx) are NOT retried
64+
* because they indicate the server received the request — retrying won't help for
65+
* auth failures (401/403), bad requests (400), or server errors (500).
66+
*/
67+
function isConnectionError(error: unknown): boolean {
68+
if (error instanceof TypeError) return true;
69+
if (error instanceof DOMException && error.name === 'AbortError') return true;
70+
// Node.js fetch may throw non-TypeError for connection issues
71+
if (error instanceof Error) {
72+
const msg = error.message.toLowerCase();
73+
return (
74+
msg.includes('econnrefused') ||
75+
msg.includes('enotfound') ||
76+
msg.includes('etimedout') ||
77+
msg.includes('econnreset') ||
78+
msg.includes('enetunreach') ||
79+
msg.includes('ehostunreach') ||
80+
msg.includes('fetch failed') ||
81+
msg.includes('dns') ||
82+
msg.includes('abort')
83+
);
84+
}
85+
return false;
86+
}
87+
88+
/**
89+
* Create a fetch wrapper that adds a per-request timeout and retries
90+
* on connection-level failures with exponential backoff.
91+
*
92+
* This prevents the Subsonic adapter from spinning indefinitely when
93+
* the server is unreachable (DNS failure, connection refused, timeout).
94+
*
95+
* @param serverUrl - The server URL, included in error messages for diagnostics
96+
* @param timeoutMs - Per-request timeout in milliseconds
97+
*/
98+
function createRetryFetch(serverUrl: string, timeoutMs: number = REQUEST_TIMEOUT_MS) {
99+
const retryFetch = async (
100+
input: string | URL | Request,
101+
init?: RequestInit
102+
): Promise<Response> => {
103+
let lastError: unknown;
104+
105+
for (let attempt = 1; attempt <= MAX_RETRIES; attempt++) {
106+
try {
107+
const controller = new AbortController();
108+
const timeoutId = setTimeout(() => controller.abort(), timeoutMs);
109+
110+
try {
111+
const response = await globalThis.fetch(input, {
112+
...init,
113+
signal: controller.signal,
114+
});
115+
clearTimeout(timeoutId);
116+
return response;
117+
} catch (error) {
118+
clearTimeout(timeoutId);
119+
throw error;
120+
}
121+
} catch (error) {
122+
lastError = error;
123+
124+
if (!isConnectionError(error)) {
125+
// Not a connection error — don't retry (e.g., HTTP errors thrown by middleware)
126+
throw error;
127+
}
128+
129+
if (attempt < MAX_RETRIES) {
130+
// Exponential backoff: 1s, 2s, 4s
131+
const delay = RETRY_BASE_DELAY_MS * Math.pow(2, attempt - 1);
132+
await new Promise((resolve) => setTimeout(resolve, delay));
133+
}
134+
}
135+
}
136+
137+
// All retries exhausted — throw a descriptive error
138+
const reason = lastError instanceof Error ? lastError.message : String(lastError);
139+
throw new SubsonicConnectionError(serverUrl, reason);
140+
};
141+
return retryFetch as typeof fetch;
142+
}
143+
144+
/**
145+
* Error thrown when the Subsonic server cannot be reached after all retry attempts.
146+
* Includes the server URL and a helpful diagnostic message for Docker/network users.
147+
*/
148+
export class SubsonicConnectionError extends Error {
149+
readonly url: string;
150+
151+
constructor(url: string, reason: string) {
152+
super(
153+
`Failed to connect to Subsonic server at ${url} after ${MAX_RETRIES} attempts. ` +
154+
`${reason}. ` +
155+
`Check that the server is running and the URL is correct. ` +
156+
`If running in Docker, ensure the container can reach the server ` +
157+
`(check DNS, network mode, and firewall settings).`
158+
);
159+
this.name = 'SubsonicConnectionError';
160+
this.url = url;
161+
}
162+
}
163+
40164
/**
41165
* Map file suffix to AudioFileType
42166
*/
@@ -160,6 +284,7 @@ export class SubsonicAdapter implements CollectionAdapter {
160284
username: config.username,
161285
password: config.password,
162286
},
287+
fetch: createRetryFetch(config.url),
163288
});
164289
}
165290

@@ -174,6 +299,10 @@ export class SubsonicAdapter implements CollectionAdapter {
174299
}
175300
this.connected = true;
176301
} catch (error) {
302+
// Re-throw SubsonicConnectionError directly (already has a descriptive message)
303+
if (error instanceof SubsonicConnectionError) {
304+
throw error;
305+
}
177306
const message = error instanceof Error ? error.message : String(error);
178307
throw new Error(`Failed to connect to Subsonic server at ${this.config.url}: ${message}`);
179308
}

packages/podkit-core/src/index.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,11 @@ export { DirectoryAdapter, createDirectoryAdapter } from './adapters/directory.j
2727
export type { DirectoryAdapterConfig, ScanProgress, ScanWarning } from './adapters/directory.js';
2828

2929
// Subsonic adapter
30-
export { SubsonicAdapter, createSubsonicAdapter } from './adapters/subsonic.js';
30+
export {
31+
SubsonicAdapter,
32+
createSubsonicAdapter,
33+
SubsonicConnectionError,
34+
} from './adapters/subsonic.js';
3135

3236
export type { SubsonicAdapterConfig } from './adapters/subsonic.js';
3337

0 commit comments

Comments
 (0)