From 485cee3612793839b3cc55ebabcf0dba3b9bd3c7 Mon Sep 17 00:00:00 2001 From: Maximilian Inckmann Date: Mon, 28 Sep 2026 20:22:46 +0200 Subject: [PATCH 1/4] addressed changes recommended during review --- README.md | 2 -- .../rendererModules/SPDXType/__tests__/SPDXType.integration.ts | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/README.md b/README.md index 54ce20d3..9c6d4d36 100644 --- a/README.md +++ b/README.md @@ -122,8 +122,6 @@ You can customize the behavior of specific renderers by passing a JSON configura **SPDXType** -- `requestTimeout` (number): Timeout in milliseconds for the SPDX license-detail fetch. - - Default: `10000`. - License details are fetched directly from the [spdx/license-list-data](https://github.com/spdx/license-list-data) raw JSON (`json/details/{ID}.json` on `raw.githubusercontent.com`). This is necessary since SPDX.org does not ship CORS headers. diff --git a/packages/stencil-library/src/rendererModules/SPDXType/__tests__/SPDXType.integration.ts b/packages/stencil-library/src/rendererModules/SPDXType/__tests__/SPDXType.integration.ts index b3ecc1e9..4c8ff81a 100644 --- a/packages/stencil-library/src/rendererModules/SPDXType/__tests__/SPDXType.integration.ts +++ b/packages/stencil-library/src/rendererModules/SPDXType/__tests__/SPDXType.integration.ts @@ -66,7 +66,7 @@ describe('SPDX license API (integration)', () => { const meaningful = await st.hasMeaningfulInformation(); expect(meaningful).toBe(true); - expect(st.licenseId).toBe('Apache-2.0'); + expect(st.data?.licenseId).toBe('Apache-2.0'); expect(st.data?.name).toBe('Apache License 2.0'); expect(st.data?.licenseId).toBe('Apache-2.0'); }, PER_TEST_TIMEOUT); From 4776b57c070a812e90618229738082b1a7cc54e8 Mon Sep 17 00:00:00 2001 From: Maximilian Inckmann Date: Tue, 29 Sep 2026 16:20:09 +0200 Subject: [PATCH 2/4] fix(Parser): gate ordered-mode committal on real resolution --- packages/stencil-library/src/utils/Parser.ts | 34 ++++++++-- .../src/utils/__tests__/Parser.unit.ts | 68 +++++++++++++++++++ 2 files changed, 97 insertions(+), 5 deletions(-) diff --git a/packages/stencil-library/src/utils/Parser.ts b/packages/stencil-library/src/utils/Parser.ts index b9dd3e9a..70dbe3af 100644 --- a/packages/stencil-library/src/utils/Parser.ts +++ b/packages/stencil-library/src/utils/Parser.ts @@ -122,16 +122,23 @@ export class Parser { const quickResult = obj.quickCheck(); if (quickResult === true) { - Parser.applySettings(obj, settings); - await obj.init(); - return obj; + if (await obj.hasMeaningfulInformation()) { + Parser.applySettings(obj, settings); + if (await this.tryInit(obj)) { + return obj; + } + continue; + } + continue; } if (quickResult === undefined || !quickResult) { if (await obj.hasMeaningfulInformation()) { Parser.applySettings(obj, settings); - await obj.init(); - return obj; + if (await this.tryInit(obj)) { + return obj; + } + continue; } } } @@ -209,6 +216,23 @@ export class Parser { return priorityScore + itemScore + actionScore; } + /** + * Runs a renderer's init() defensively so that a thrown error inside a + * renderer after a successful probe cannot abort the whole detection. + * Returns true if init() completed without throwing, false otherwise. + * @param obj The renderer instance to initialize + * @returns {Promise} Whether initialization completed without throwing + */ + private static async tryInit(obj: GenericIdentifierType): Promise { + try { + await obj.init(); + return true; + } catch (e) { + console.error('Renderer init() failed, skipping:', e); + return false; + } + } + /** * Applies settings to a renderer object. * @param obj The renderer instance to apply settings to diff --git a/packages/stencil-library/src/utils/__tests__/Parser.unit.ts b/packages/stencil-library/src/utils/__tests__/Parser.unit.ts index 7ccf0115..20997413 100644 --- a/packages/stencil-library/src/utils/__tests__/Parser.unit.ts +++ b/packages/stencil-library/src/utils/__tests__/Parser.unit.ts @@ -294,6 +294,74 @@ describe('Parser', () => { expect(result!.getSettingsKey()).toBe('DOIType'); }); + it('in ordered mode, does NOT commit a fully-quick renderer when its probe fails', async () => { + // ORCIDType matches format (quickResult=true) but the network/API probe + // fails. With fallbackToAll=false the lookup must not commit it. + mockRenderers[1].constructor = createMockConstructor({ + key: 'ORCIDType', + quickResult: true, + meaningfulInfoResult: false, + }); + + const result = await Parser.getBestFit('value', emptySettings, ['ORCIDType'], false); + expect(result).toBeNull(); + }); + + it('in ordered mode, falls through to the next listed renderer when the probe fails', async () => { + // First listed renderer matches format but its probe fails; the next one + // matches and resolves. The first must not win on a bare format match. + mockRenderers[1].constructor = createMockConstructor({ + key: 'ORCIDType', + quickResult: true, + meaningfulInfoResult: false, + }); + mockRenderers[4].constructor = createMockConstructor({ + key: 'FallbackType', + quickResult: true, + meaningfulInfoResult: true, + }); + + const result = await Parser.getBestFit('value', emptySettings, ['ORCIDType', 'FallbackType'], false); + expect(result).not.toBeNull(); + expect(result!.getSettingsKey()).toBe('FallbackType'); + }); + + it('in ordered mode with fallbackToAll=true falls back to the full registry when a listed probe fails', async () => { + // ORCIDType is listed and quick-matches, but its probe fails. With + // fallback the full registry is retried and the highest-priority + // meaningful candidate (DOIType) wins. + mockRenderers[1].constructor = createMockConstructor({ + key: 'ORCIDType', + quickResult: true, + meaningfulInfoResult: false, + }); + + const result = await Parser.getBestFit('value', emptySettings, ['ORCIDType'], true); + expect(result).not.toBeNull(); + expect(result!.getSettingsKey()).toBe('DOIType'); + }); + + it('in ordered mode, a renderer whose init() rejects does not abort the lookup', async () => { + // Simulate the reviewer's literal concern: a fully-quick renderer probes + // successfully but then throws during init(). This must not propagate. + const throwingInit = vi.fn().mockImplementation(function(value: string) { + return { + value, + quickCheck: vi.fn().mockReturnValue(true), + hasMeaningfulInformation: vi.fn().mockResolvedValue(true), + init: vi.fn().mockRejectedValue(new Error('boom')), + getSettingsKey: vi.fn().mockReturnValue('ORCIDType'), + settings: undefined as unknown, + items: [], + actions: [], + }; + }); + mockRenderers[1].constructor = throwingInit; + + // The rejection must be swallowed rather than aborting the lookup. + await expect(Parser.getBestFit('value', emptySettings, ['ORCIDType'], false)).resolves.toBeNull(); + }); + it('warns on settings error but still returns the renderer', async () => { const badConstructor = vi.fn().mockImplementation(function(value: string) { return { From f55cf9920d2826b16510edd45e825d32658f4e47 Mon Sep 17 00:00:00 2001 From: Maximilian Inckmann Date: Tue, 29 Sep 2026 16:22:17 +0200 Subject: [PATCH 3/4] fix(Parser): gate getEstimatedPriority on real resolution too getEstimatedPriority() had the same flaw as ordered mode: it claimed a renderer whose quickCheck() returned true without confirming it actually resolves via hasMeaningfulInformation(). It feeds auto-detection priority scoring, so a format-only match could be ranked without the resource being real. Apply the same resolution gate used by default and ordered modes. --- packages/stencil-library/src/utils/Parser.ts | 5 ++++- .../src/utils/__tests__/Parser.unit.ts | 20 +++++++++++++++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/packages/stencil-library/src/utils/Parser.ts b/packages/stencil-library/src/utils/Parser.ts index 70dbe3af..8b81f959 100644 --- a/packages/stencil-library/src/utils/Parser.ts +++ b/packages/stencil-library/src/utils/Parser.ts @@ -38,7 +38,10 @@ export class Parser { const obj = new renderers[i].constructor(value); const quickResult = obj.quickCheck(); if (quickResult === true) { - return i; + const hasMeaningful = await obj.hasMeaningfulInformation(); + if (hasMeaningful) { + return i; + } } if (quickResult === undefined) { const hasMeaningful = await obj.hasMeaningfulInformation(); diff --git a/packages/stencil-library/src/utils/__tests__/Parser.unit.ts b/packages/stencil-library/src/utils/__tests__/Parser.unit.ts index 20997413..c307b2ab 100644 --- a/packages/stencil-library/src/utils/__tests__/Parser.unit.ts +++ b/packages/stencil-library/src/utils/__tests__/Parser.unit.ts @@ -399,12 +399,28 @@ describe('Parser', () => { expect(priority).toBe(1); }); - it('returns 0 when the first renderer matches', async () => { - mockRenderers[0].constructor = createMockConstructor({ key: 'DateType', quickResult: true }); + it('returns 0 when the first renderer matches and resolves', async () => { + mockRenderers[0].constructor = createMockConstructor({ + key: 'DateType', + quickResult: true, + meaningfulInfoResult: true, + }); const priority = await Parser.getEstimatedPriority('value'); expect(priority).toBe(0); }); + it('skips a quick-match renderer whose network probe fails', async () => { + // DateType (0) matches format but its probe fails; ORCIDType (1) + // matches format and resolves. A format match alone must not win. + mockRenderers[0].constructor = createMockConstructor({ + key: 'DateType', + quickResult: true, + meaningfulInfoResult: false, + }); + const priority = await Parser.getEstimatedPriority('value'); + expect(priority).toBe(1); + }); + it('falls back to async check when quick returns undefined', async () => { // Make everything fail except DOIType (index 2) which is async-only mockRenderers.forEach(r => { From 3d47243bb65f55ac7fcc02197d90472c4d9223ab Mon Sep 17 00:00:00 2001 From: Maximilian Inckmann Date: Tue, 29 Sep 2026 16:24:33 +0200 Subject: [PATCH 4/4] fix(Parser): never let a throwing renderer blank the view A renderer that throws inside init() after a successful probe (most notably Handle/ORCID, which re-fetch with no catch) previously propagated up through IndexedDBUtil.getEntity() and blanked the whole component. Wrap init() defensively: - Parser default mode now uses tryInit() like ordered mode; it still returns the best candidate so the identifier stays visible instead of blanking. - IndexedDBUtil.getEntity() fresh path guards init() and returns the renderer on throw (skipping the cache) so it is not treated as unmatched and hidden. During loading the PID candidate continues to render as plaintext and is only replaced once data is ready, so users are never shown a blank/error. --- .../src/utils/IndexedDBUtil.ts | 22 ++++++++++++++----- packages/stencil-library/src/utils/Parser.ts | 7 +++++- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/packages/stencil-library/src/utils/IndexedDBUtil.ts b/packages/stencil-library/src/utils/IndexedDBUtil.ts index f2851063..3f57bf1d 100644 --- a/packages/stencil-library/src/utils/IndexedDBUtil.ts +++ b/packages/stencil-library/src/utils/IndexedDBUtil.ts @@ -214,12 +214,24 @@ export class Database { } renderer.settings = settings.find(value => value.type === renderer.getSettingsKey())?.values; - await renderer.init(); - // Only cache in IndexedDB if the renderer resolved successfully. - // This prevents false positives (e.g. SPDX regex matching a random word) - // from being persisted and polluting the cache. - if (renderer.isResolvable()) { + // Guard init() so a renderer that throws during a fresh fetch (e.g. a + // timeout surfacing mid-render) cannot propagate up to the component and + // blank the whole view. We still return the renderer so the identifier + // stays visible (its preview renders from the probe data) rather than + // being treated as unmatched/hidden; we simply skip caching it. + let initOk = true; + try { + await renderer.init(); + } catch (error) { + initOk = false; + console.error('Could not initialize renderer', renderer.getSettingsKey(), error); + } + + // Only cache in IndexedDB if the renderer initialized without throwing and + // resolved successfully. This prevents false positives (e.g. SPDX regex + // matching a random word) from being persisted and polluting the cache. + if (initOk && renderer.isResolvable()) { await this.addEntity(renderer, orderedRendererKeys); console.debug('added entity to db', value, renderer); } else { diff --git a/packages/stencil-library/src/utils/Parser.ts b/packages/stencil-library/src/utils/Parser.ts index 8b81f959..f2a5a157 100644 --- a/packages/stencil-library/src/utils/Parser.ts +++ b/packages/stencil-library/src/utils/Parser.ts @@ -196,7 +196,12 @@ export class Parser { Parser.applySettings(best, settings); - await best.init(); + // Guard init() so a renderer that throws after a successful probe (e.g. + // a timeout surfacing mid-render) cannot abort the whole lookup. Unlike + // the ordered branch (where we fall through to the next candidate), here + // we still return the best candidate so the view stays populated rather + // than being blanked/hidden. + await this.tryInit(best); return best; }