Fix databook validate: ESM require crash, silent engine fallback, error masking#1
Merged
Merged
Conversation
…or masking
- Hoist require('fs')/require('os')/require('path') in executeLocalValidation
to top-level ESM imports (package is type:module; bare require() throws).
- executeValidation no longer falls into executeLocalValidation with a null
engine when neither a local engine nor --endpoint is available; dies with
a clear message instead.
- executeLocalValidation's catch no longer returns e.message as if it were
a SHACL report when execFileSync fails with no captured stdout.
Reported and diagnosed by Ben Wortley (UN-GGCE QA pass); confirmed against
UN-GGCE suite (capability-maturity, four workflow DataBooks, satellite-orbits)
all validating CONFORMS against ggsc-shacl-all with the fix applied.
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
1 participant
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.
Fixes three stacked issues in
commands/validate.js, found and diagnosed by Ben Wortley while running SHACL validation for the UN-GGCE QA pass:executeLocalValidation()used bare CommonJSrequire('fs')/require('os')/require('path'), but the package is"type": "module"— throwsrequire is not definedthe moment validation reaches the local engine.--dry-runskips this path, which is why it went unnoticed.executeValidation()hadif (jenaShacl || !endpointOpt)— when no engine is installed and no--endpointis given, it still called local validation with anullengine instead of failing informatively.execFileSyncreturnede.messageas the result, so past bug Fix databook validate: ESM require crash, silent engine fallback, error masking #1, a missing engine would come back looking like a SHACL report instead of an error.Fix hoists the three
requires to top-level ESM imports and dies with a clear message when no engine is found and no--endpointis given.Ben applied this fix locally, put
pyshaclbehind it, and re-ran the full UN-GGCE suite — all six DataBooks (capability-maturity, the four workflow DataBooks, satellite-orbits) validate CONFORMS againstggsc-shacl-all.Also worth noting for a follow-up: a blank line between a
<!-- databook:id: ... -->annotation and its code fence makes the block anonymous, and anonymous blocks get PUT to the default graph sequentially, each replacing the last. Bit the UN-GGCE QA pass twice (satellite-orbits in June, capability-maturity this week). Worth the parser either tolerating the blank line or warning loudly when a block resolves to the default graph — not addressed in this PR.