Robustness fix for renderer instantiation errors - #454
Merged
Merged
Conversation
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.
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.
🎨 Chromatic Visual Tests✅ No visual changes ReviewChromatic provides automated visual testing and review for component changes. |
🎨 Chromatic Visual Tests✅ No visual changes ReviewChromatic provides automated visual testing and review for component changes. |
Test ResultsCoverage Summary
Coverage & Quality Reports📊 Codecov Report For more details, check the workflow run |
Test ResultsCoverage Summary
Coverage & Quality Reports📊 Codecov Report For more details, check the workflow run |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #454 +/- ##
==========================================
+ Coverage 80.69% 80.77% +0.08%
==========================================
Files 57 57
Lines 3787 3803 +16
Branches 1147 1172 +25
==========================================
+ Hits 3056 3072 +16
Misses 727 727
Partials 4 4
🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Rejected probes can still abort selection, and ordered initialization failures can hide the identifier.
Review effort: Balanced
Findings: 4
Open (4)
What changed in this PR
This PR aims to keep the PID component visible when a renderer cannot initialize, continuing the SPDX-related work in #438.
Changes:
- Checks whether quick-matching renderers have meaningful data and guards initialization failures.
- Avoids caching a renderer when initialization fails.
- Updates parser tests, an SPDX integration assertion, and SPDX documentation.
| File | Description |
|---|---|
| README.md | Removes the documented SPDX request timeout. |
| packages/stencil-library/src/utils/Parser.ts | Changes renderer selection and guards initialization. |
| packages/stencil-library/src/utils/IndexedDBUtil.ts | Returns failed-init renderers without caching them. |
| packages/stencil-library/src/utils/__tests__/Parser.unit.ts | Adds ordered-selection and probe tests. |
| packages/stencil-library/src/rendererModules/SPDXType/__tests__/SPDXType.integration.ts | Updates a license ID assertion. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+41
to
47
| const hasMeaningful = await obj.hasMeaningfulInformation(); | ||
| if (hasMeaningful) { | ||
| return i; | ||
| } | ||
| } | ||
| if (quickResult === undefined) { | ||
| const hasMeaningful = await obj.hasMeaningfulInformation(); |
| Parser.applySettings(obj, settings); | ||
| await obj.init(); | ||
| return obj; | ||
| if (await obj.hasMeaningfulInformation()) { |
| await obj.init(); | ||
| return obj; | ||
| if (await obj.hasMeaningfulInformation()) { | ||
| Parser.applySettings(obj, settings); |
| if (await this.tryInit(obj)) { | ||
| return obj; | ||
| } | ||
| continue; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a continuation of #438. It ensures that the pid-component shows something if a renderer fails during init or hasMeaningfulInformation, i.e., It somehow isn't able to load meaningful information.