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); 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 b9dd3e9a..f2a5a157 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(); @@ -122,16 +125,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; } } } @@ -186,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; } @@ -209,6 +224,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..c307b2ab 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 { @@ -331,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 => {