mirror of
https://github.com/Psychotoxical/psysonic.git
synced 2026-07-21 14:55:43 +00:00
fix(music-network): surface the underlying cause of a connect NETWORK error (#1285)
* fix(music-network): surface the underlying cause of a connect NETWORK error NETWORK is the transport catch-all: a DNS failure, a TLS handshake broken by a proxy or AV, a timeout and an unrecognised provider API error all collapse into it. The connect form rendered only the translated string, so the cause never reached the user or a bug report -- the same wire, transport and token-poll strategy work for one provider and fail for another with no way to tell why (#1283). Add errorDetail(), which carries the transport message for NETWORK only (capped), and append it to the message the connect form shows. * docs(changelog): add entry for PR #1285 * fix(music-network): make the connect error line selectable so it can be copied
This commit is contained in:
@@ -302,6 +302,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
* Internet Radio playback stayed on HTML5 after v1.32, but EQ changes only reached the Rust engine used for library tracks — toggling EQ or switching presets had no effect on a live station. Radio now routes through a Web Audio 10-band graph on the same `<audio>` element when EQ is enabled; preset and slider changes update filters in place without restarting the stream.
|
||||
|
||||
### Music Network — connect errors now name their cause
|
||||
|
||||
**By [@Psychotoxical](https://github.com/Psychotoxical), PR [#1285](https://github.com/Psychotoxical/psysonic/pull/1285)**
|
||||
|
||||
* Connecting a scrobble service could fail with only "Network error — check your connection or URL", which covers everything from a DNS failure to a blocked host, an interrupted TLS handshake or a rejected request. The underlying error is now shown alongside it, so a failing connect can be told apart from a reachability problem on your machine or network.
|
||||
|
||||
|
||||
## [1.49.0] - 2026-06-29
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { useState } from 'react';
|
||||
import { useTranslation } from 'react-i18next';
|
||||
import {
|
||||
errorDetail,
|
||||
errorI18nKey,
|
||||
isMusicNetworkError,
|
||||
listPresets,
|
||||
@@ -33,8 +34,12 @@ export function ConnectProviderForm({
|
||||
p => !(p.manifest.credentials === 'bundled' && connectedPresetIds.includes(p.manifest.presetId)),
|
||||
);
|
||||
|
||||
const toMessage = (e: unknown): string =>
|
||||
isMusicNetworkError(e) ? t(errorI18nKey(e.code)) : t('musicNetwork.connectFailed');
|
||||
const toMessage = (e: unknown): string => {
|
||||
if (!isMusicNetworkError(e)) return t('musicNetwork.connectFailed');
|
||||
const message = t(errorI18nKey(e.code));
|
||||
const detail = errorDetail(e);
|
||||
return detail ? `${message} (${detail})` : message;
|
||||
};
|
||||
|
||||
const run = async (presetId: PresetId, payload: Record<string, string>) => {
|
||||
// Enforce the manifest's `required` fields client-side so an empty URL/token
|
||||
@@ -126,7 +131,9 @@ export function ConnectProviderForm({
|
||||
</div>
|
||||
);
|
||||
})}
|
||||
{error && <p style={{ fontSize: 12, color: 'var(--danger)' }}>{error}</p>}
|
||||
{/* Selectable: the transport detail in a NETWORK error is what a bug report
|
||||
needs verbatim, and the app disables text selection everywhere else. */}
|
||||
{error && <p data-selectable style={{ fontSize: 12, color: 'var(--danger)' }}>{error}</p>}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,36 @@
|
||||
// The NETWORK code is the transport catch-all — a DNS failure, a broken TLS
|
||||
// handshake and an unrecognised provider API error all land on it, so its
|
||||
// translated string alone tells a reporter (and us) nothing. These tests pin
|
||||
// that the underlying cause travels with it, and only with it: every other code
|
||||
// already names its failure and must stay clean.
|
||||
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { MusicNetworkError, errorDetail, errorI18nKey } from './errors';
|
||||
|
||||
describe('errorI18nKey', () => {
|
||||
it('maps a code to its namespaced key', () => {
|
||||
expect(errorI18nKey('NETWORK')).toBe('musicNetwork.errors.NETWORK');
|
||||
});
|
||||
});
|
||||
|
||||
describe('errorDetail', () => {
|
||||
it('returns the transport message for a NETWORK error', () => {
|
||||
const e = new MusicNetworkError('NETWORK', 'error sending request for url (https://ws.audioscrobbler.com/2.0/)');
|
||||
expect(errorDetail(e)).toBe('error sending request for url (https://ws.audioscrobbler.com/2.0/)');
|
||||
});
|
||||
|
||||
it('trims surrounding whitespace', () => {
|
||||
expect(errorDetail(new MusicNetworkError('NETWORK', ' dns error '))).toBe('dns error');
|
||||
});
|
||||
|
||||
it('caps an overlong message so it cannot blow up the form', () => {
|
||||
const detail = errorDetail(new MusicNetworkError('NETWORK', 'x'.repeat(500)));
|
||||
expect(detail).toHaveLength(200);
|
||||
});
|
||||
|
||||
it('stays empty for codes whose own message is already specific', () => {
|
||||
expect(errorDetail(new MusicNetworkError('AUTH_SESSION_INVALID', 'Audioscrobbler 9 Invalid session key'))).toBe('');
|
||||
expect(errorDetail(new MusicNetworkError('AUTH_TIMEOUT', 'Authorization timed out'))).toBe('');
|
||||
expect(errorDetail(new MusicNetworkError('CUSTOM_URL_INVALID', 'bad url'))).toBe('');
|
||||
});
|
||||
});
|
||||
@@ -43,3 +43,21 @@ export function isMusicNetworkError(e: unknown): e is MusicNetworkError {
|
||||
export function errorI18nKey(code: MusicNetworkErrorCode): string {
|
||||
return `musicNetwork.errors.${code}`;
|
||||
}
|
||||
|
||||
/** Longest transport detail we append to a translated message. */
|
||||
const DETAIL_MAX_LEN = 200;
|
||||
|
||||
/**
|
||||
* The transport detail behind a NETWORK error, for display next to the
|
||||
* translated message.
|
||||
*
|
||||
* NETWORK is the catch-all: a DNS failure, a TLS handshake broken by a proxy or
|
||||
* AV, a timeout and a provider API error the auth classifier did not recognise
|
||||
* all collapse into it. Its i18n string therefore cannot say what went wrong,
|
||||
* and a user report that quotes only that string is not actionable. Every other
|
||||
* code names its own failure, so it needs no detail.
|
||||
*/
|
||||
export function errorDetail(e: MusicNetworkError): string {
|
||||
if (e.code !== 'NETWORK') return '';
|
||||
return e.message.trim().slice(0, DETAIL_MAX_LEN);
|
||||
}
|
||||
|
||||
@@ -30,7 +30,7 @@ export type {
|
||||
CapabilityState,
|
||||
CapabilityStatus,
|
||||
} from './core/capabilities';
|
||||
export { MusicNetworkError, errorI18nKey, isMusicNetworkError } from './core/errors';
|
||||
export { MusicNetworkError, errorDetail, errorI18nKey, isMusicNetworkError } from './core/errors';
|
||||
export type { MusicNetworkErrorCode } from './core/errors';
|
||||
export type {
|
||||
ArtistStats,
|
||||
|
||||
Reference in New Issue
Block a user