From a1906fe15ae2d96007838cc1572120a84f921b33 Mon Sep 17 00:00:00 2001 From: Selman Trabzon Date: Mon, 12 Oct 2026 22:23:40 +0300 Subject: [PATCH 1/2] fix(simkl): show why Simkl rejected a link or Client ID Linking Simkl failed with only "Request failed with status code 400" (#8): the PIN routes passed axios's generic message on and logged nothing, so neither the UI nor the server log said what Simkl (or a proxy in front of it) objected to. - SimklAPI wraps rejected requests in SimklRequestError, carrying Simkl's `error`/`message` (or the text of a proxy's error page). - The PIN code/token routes log their failures. - validateClientId uses /oauth/pin, the only endpoint found to check the Client ID; the CDN catalog accepted any value. A rejected Client ID now fails the admin save with Simkl's reason, and the health check logs why it degraded. Refs #8 Co-Authored-By: Claude Opus 5.5 --- server/api/simkl/index.test.ts | 77 ++++++++++++++++++++++++++++++ server/api/simkl/index.ts | 46 ++++++++++++++++-- server/lib/integrationHealth.ts | 6 ++- server/routes/settings/index.ts | 4 +- server/routes/user/usersettings.ts | 8 ++++ 5 files changed, 136 insertions(+), 5 deletions(-) create mode 100644 server/api/simkl/index.test.ts diff --git a/server/api/simkl/index.test.ts b/server/api/simkl/index.test.ts new file mode 100644 index 0000000000..51d2479f9b --- /dev/null +++ b/server/api/simkl/index.test.ts @@ -0,0 +1,77 @@ +import SimklAPI, { + SimklRequestError, + simklErrorDetail, +} from '@server/api/simkl'; +import axios, { AxiosError, type AxiosAdapter } from 'axios'; +import assert from 'node:assert/strict'; +import { afterEach, describe, it } from 'node:test'; + +const previousAxiosAdapter = axios.defaults.adapter; + +afterEach(() => { + axios.defaults.adapter = previousAxiosAdapter; +}); + +/** The client builds its axios instance on construction, so stub first. */ +const clientAnswering = (status: number, data: unknown) => { + const urls: string[] = []; + axios.defaults.adapter = (async (config) => { + urls.push(String(config.url)); + const response = { + status, + statusText: '', + headers: {}, + config, + data, + }; + if (status < 400) return response; + throw new AxiosError( + `Request failed with status code ${status}`, + AxiosError.ERR_BAD_REQUEST, + config, + undefined, + response + ); + }) as AxiosAdapter; + return { client: new SimklAPI({ clientId: 'test-client' }), urls }; +}; + +describe('SimklAPI errors', () => { + it('keeps Simkl’s own reason for a rejected request', async () => { + const { client } = clientAnswering(412, { + error: 'client_id_failed', + code: 412, + message: 'Your client_id is wrong. Try another one', + }); + await assert.rejects(client.requestPinCode(), (error: unknown) => { + assert.ok(error instanceof SimklRequestError); + assert.equal(error.status, 412); + assert.equal( + error.message, + 'Simkl rejected the request (HTTP 412): client_id_failed — Your client_id is wrong. Try another one' + ); + return true; + }); + }); + + it('reduces a proxy’s HTML error page to its text', () => { + assert.equal( + simklErrorDetail( + '

400 Bad Request

\n

Invalid header

' + ), + '400 Bad Request Invalid header' + ); + assert.equal(simklErrorDetail(''), undefined); + assert.equal(simklErrorDetail({}), undefined); + }); + + it('validates the Client ID against the PIN endpoint', async () => { + const { client, urls } = clientAnswering(200, { + result: 'OK', + user_code: 'ABCDE', + }); + await client.validateClientId(); + assert.equal(urls.length, 1); + assert.match(urls[0], /^\/oauth\/pin\?client_id=test-client&/); + }); +}); diff --git a/server/api/simkl/index.ts b/server/api/simkl/index.ts index 81058e5488..657b8a45c3 100644 --- a/server/api/simkl/index.ts +++ b/server/api/simkl/index.ts @@ -49,6 +49,38 @@ export class SimklTemporarilyUnavailableError extends Error { } } +/** Simkl (or a proxy on the way) refused the request; keeps its explanation. */ +export class SimklRequestError extends Error { + constructor( + public readonly status: number, + detail?: string, + cause?: unknown + ) { + super( + `Simkl rejected the request (HTTP ${status})${detail ? `: ${detail}` : ''}` + ); + this.name = 'SimklRequestError'; + this.cause = cause; + } +} + +/** Simkl answers `{ error, message }`; proxies tend to answer HTML or text. */ +export const simklErrorDetail = (data: unknown): string | undefined => { + if (data && typeof data === 'object') { + const { message, error } = data as { message?: unknown; error?: unknown }; + const parts = [error, message].filter( + (part): part is string => typeof part === 'string' && part.length > 0 + ); + return parts.length ? parts.join(' — ') : undefined; + } + if (typeof data !== 'string') return undefined; + const text = data + .replace(/<[^>]*>/g, ' ') + .replace(/\s+/g, ' ') + .trim(); + return text ? text.slice(0, 200) : undefined; +}; + type SimklOptions = { clientId?: string; accessToken?: string; @@ -138,14 +170,22 @@ export default class SimklAPI extends ExternalAPI { } if (!status || status >= 500) throw new SimklTemporarilyUnavailableError(error); - throw error; + throw new SimklRequestError( + status, + simklErrorDetail(error.response?.data), + error + ); } } } - /** Requires a real Client ID; `/movies/trending` succeeds without one. */ + /** + * The PIN endpoint is the only one found to check the Client ID (412 + * `client_id_failed`); catalogs and title lookups accept any value. The + * code it issues just expires unused. + */ public async validateClientId(): Promise { - await this.getCdnCatalog('/discover/trending/tv/week_100.json'); + await this.requestPinCode(); } public async requestPinCode(): Promise { diff --git a/server/lib/integrationHealth.ts b/server/lib/integrationHealth.ts index 34aa3a4489..17983fec64 100644 --- a/server/lib/integrationHealth.ts +++ b/server/lib/integrationHealth.ts @@ -237,7 +237,11 @@ const checkSimkl = async (): Promise => { try { await new SimklAPI().validateClientId(); return healthy('Simkl is reachable and the Client ID was accepted.'); - } catch { + } catch (e) { + logger.warn('Simkl health check failed', { + label: 'Integration Health', + errorMessage: e instanceof Error ? e.message : 'unknown error', + }); return degraded('Simkl could not accept the saved Client ID.'); } }; diff --git a/server/routes/settings/index.ts b/server/routes/settings/index.ts index 1a548186c8..6237c88e34 100644 --- a/server/routes/settings/index.ts +++ b/server/routes/settings/index.ts @@ -3,7 +3,7 @@ import type { JellyfinLibrary } from '@server/api/jellyfin'; import JellyfinAPI from '@server/api/jellyfin'; import PlexAPI from '@server/api/plexapi'; import PlexTvAPI from '@server/api/plextv'; -import SimklAPI from '@server/api/simkl'; +import SimklAPI, { SimklRequestError } from '@server/api/simkl'; import TautulliAPI from '@server/api/tautulli'; import TraktAPI from '@server/api/trakt'; import { ApiErrorCode } from '@server/constants/error'; @@ -749,6 +749,8 @@ settingsRoutes.post('/simkl', async (req, res, next) => { label: 'Settings', errorMessage: e instanceof Error ? e.message : 'unknown error', }); + if (e instanceof SimklRequestError) + return next({ status: 400, message: e.message }); return next({ status: 500, message: 'Unable to save Simkl settings.' }); } }); diff --git a/server/routes/user/usersettings.ts b/server/routes/user/usersettings.ts index 463179e92f..760ef1e147 100644 --- a/server/routes/user/usersettings.ts +++ b/server/routes/user/usersettings.ts @@ -1148,6 +1148,10 @@ userSettingsRoutes.post<{ id: string }>( deviceCode: userCode, }); } catch (e) { + logger.error('Failed to start Simkl PIN authorization', { + label: 'API', + errorMessage: e instanceof Error ? e.message : 'unknown error', + }); return res.status(e instanceof SimklNotConfiguredError ? 400 : 500).json({ message: e instanceof Error @@ -1223,6 +1227,10 @@ userSettingsRoutes.post<{ id: string }>( } catch (e) { if (e instanceof SimklAccountAlreadyLinkedError) return res.status(409).json({ message: e.message }); + logger.error('Failed to complete Simkl PIN authorization', { + label: 'API', + errorMessage: e instanceof Error ? e.message : 'unknown error', + }); return res.status(e instanceof SimklNotConfiguredError ? 400 : 500).json({ message: e instanceof Error From 3b7d64444f46947c07a3a101690e3a565f721a53 Mon Sep 17 00:00:00 2001 From: Selman Trabzon Date: Mon, 12 Oct 2026 22:28:38 +0300 Subject: [PATCH 2/2] fix(simkl): cap Simkl's JSON error reason like a text one Co-Authored-By: Claude Opus 5.5 --- server/api/simkl/index.test.ts | 7 +++++++ server/api/simkl/index.ts | 24 ++++++++++++++---------- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/server/api/simkl/index.test.ts b/server/api/simkl/index.test.ts index 51d2479f9b..118de3726b 100644 --- a/server/api/simkl/index.test.ts +++ b/server/api/simkl/index.test.ts @@ -65,6 +65,13 @@ describe('SimklAPI errors', () => { assert.equal(simklErrorDetail({}), undefined); }); + it('caps a long JSON reason like a text one', () => { + assert.equal( + simklErrorDetail({ error: 'bad', message: 'x'.repeat(500) })?.length, + 200 + ); + }); + it('validates the Client ID against the PIN endpoint', async () => { const { client, urls } = clientAnswering(200, { result: 'OK', diff --git a/server/api/simkl/index.ts b/server/api/simkl/index.ts index 657b8a45c3..16b56d12ae 100644 --- a/server/api/simkl/index.ts +++ b/server/api/simkl/index.ts @@ -64,21 +64,25 @@ export class SimklRequestError extends Error { } } +const SIMKL_ERROR_DETAIL_LIMIT = 200; + /** Simkl answers `{ error, message }`; proxies tend to answer HTML or text. */ export const simklErrorDetail = (data: unknown): string | undefined => { + let text = ''; if (data && typeof data === 'object') { const { message, error } = data as { message?: unknown; error?: unknown }; - const parts = [error, message].filter( - (part): part is string => typeof part === 'string' && part.length > 0 - ); - return parts.length ? parts.join(' — ') : undefined; + text = [error, message] + .filter( + (part): part is string => typeof part === 'string' && part.length > 0 + ) + .join(' — '); + } else if (typeof data === 'string') { + text = data + .replace(/<[^>]*>/g, ' ') + .replace(/\s+/g, ' ') + .trim(); } - if (typeof data !== 'string') return undefined; - const text = data - .replace(/<[^>]*>/g, ' ') - .replace(/\s+/g, ' ') - .trim(); - return text ? text.slice(0, 200) : undefined; + return text ? text.slice(0, SIMKL_ERROR_DETAIL_LIMIT) : undefined; }; type SimklOptions = {