Improve error message information across all possible errors by jviotti · Pull Request #777 · sourcemeta/one

jviotti · 2026-03-25T16:57:04Z

Signed-off-by: Juan Cruz Viotti jv@jviotti.com

Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>

jviotti · 2026-03-25T16:59:05Z

test/cli/index/common/fail-bundle-ref-no-fragment.sh

-  https://sourcemeta.com/example/schemas/test#foo
-    at schema location "/allOf/0/\$ref"
+  at identifier https://sourcemeta.com/example/schemas/test#foo
+  at schema location "/allOf/0/\$ref"


Here we don't have a path to know WHICH schema originated the reference?

Also, if we have the original schema path, given the pointer here, we could derive the line/column positions

jviotti · 2026-03-25T16:59:50Z

test/cli/index/common/fail-bundle-ref-no-fragment.sh

 error: Could not resolve schema reference
-  https://sourcemeta.com/example/schemas/test#foo
-    at schema location "/allOf/0/\$ref"
+  at identifier https://sourcemeta.com/example/schemas/test#foo


Suggested change

at identifier https://sourcemeta.com/example/schemas/test#foo

to identifier https://sourcemeta.com/example/schemas/test#foo

To better say this is the target

jviotti · 2026-03-25T17:00:15Z

test/cli/index/common/fail-circular-config-extends.sh

-  at "/extends/extends"
-  to $(realpath "$TMP")/a.json
+  from path $(realpath "$TMP")/b.json
+  at location "/extends/extends"


If we have the path and the pointer location, we can derive line/column info?

jviotti · 2026-03-25T17:01:15Z

test/cli/index/common/fail-circular-config-include.sh

-  at "/contents/x/include/include"
-  to $(realpath "$TMP")/one.json
+  from path $(realpath "$TMP")/other.json
+  at location "/contents/x/include/include"


Same here. We can do line/column

jviotti · 2026-03-25T17:02:27Z

test/cli/index/common/fail-collection-path-dot.sh

 Detecting: $(realpath "$TMP")/one.json (#1)
 (100%) Resolving: one.json
 error: Could not determine the dialect of the schema
+  at path $(realpath "$TMP")/one.json


I think this is hiding a deeper issue: this should be a success scenario as we make the detection IGNORE any known configuration file? Either the main one or any of the ones it included/extended?

jviotti · 2026-03-25T17:02:41Z

test/cli/index/common/fail-collection-path-empty.sh

 Detecting: $(realpath "$TMP")/one.json (#1)
 (100%) Resolving: one.json
 error: Could not determine the dialect of the schema
+  at path $(realpath "$TMP")/one.json


I think this is hiding a deeper issue: this should be a success scenario as we make the detection IGNORE any known configuration file? Either the main one or any of the ones it included/extended?

jviotti · 2026-03-25T17:02:49Z

test/cli/index/common/fail-config-extends-itself.sh

-  at "/extends"
-  to $(realpath "$TMP")/one.json
+  from path $(realpath "$TMP")/one.json
+  at location "/extends"


We can do line/column

jviotti · 2026-03-25T17:03:13Z

test/cli/index/common/fail-config-include-chain-cycle.sh

-  at "/contents/x/include/include/include"
-  to $(realpath "$TMP")/b.json
+  from path $(realpath "$TMP")/c.json
+  at location "/contents/x/include/include/include"


Add line/column

jviotti · 2026-03-25T17:03:31Z

test/cli/index/common/fail-config-include-itself.sh

-  at "/contents/x/include/include"
-  to $(realpath "$TMP")/self.json
+  from path $(realpath "$TMP")/self.json
+  at location "/contents/x/include/include"


Add line/column

jviotti · 2026-03-25T17:03:46Z

test/cli/index/common/fail-dependents-remove-referenced-schema.sh

@@ -1,15 +1,11 @@
 #!/bin/sh
-


Non-sense removal of blank lines?

test/cli/index/common/fail-dependents-remove-referenced-schema.sh

jviotti · 2026-03-25T17:04:55Z

test/cli/index/common/fail-extend-builtin-not-found.sh

-  at "/extends"
-  to @foo/bar
+  from path $(realpath "$TMP")/one.json
+  at location "/extends"


Add line/column

jviotti · 2026-03-25T17:05:08Z

test/cli/index/common/fail-extend-file-not-found.sh

-  at "/extends"
-  to $(realpath "$TMP")/nonexistent.json
+  from path $(realpath "$TMP")/one.json
+  at location "/extends"


Add line/column

jviotti · 2026-03-25T17:05:17Z

test/cli/index/common/fail-external-reference.sh

@@ -1,12 +1,9 @@
 #!/bin/sh
-


Non-sense removal of blank lines

test/cli/index/common/fail-external-reference.sh

jviotti · 2026-03-25T17:06:05Z

test/cli/index/common/fail-id-outside-base.sh

 (100%) Resolving: test.json
 error: The schema identifier is not relative to the corresponding base
-  at https://other.com/test
+  at identifier https://other.com/test


We are missing the schema path that caused this

test/cli/index/common/fail-invalid-schema-top-level-ref-draft7.sh

jviotti · 2026-03-25T17:07:13Z

test/cli/index/common/fail-invalid-schema-uri-percentage.sh

 (100%) Resolving: test.json
 error: The input is not a valid URI
+  at path $(realpath "$TMP")/schemas/test.json
  at column 25


But which line? Maybe also have a schema location?

jviotti · 2026-03-25T17:08:08Z

test/cli/index/common/fail-output-non-directory.sh

 cat << EOF > "$TMP/expected.txt"
-error: file already exists
-  at $(realpath "$TMP")/output
+error: File already exists


can we say something like and its not a directory

test/cli/index/common/fail-schema-non-string-ref.sh

jviotti · 2026-03-25T17:08:39Z

test/cli/index/common/fail-schema-self-metaschema.sh

@@ -1,12 +1,9 @@
 #!/bin/sh
-


Non-sense line removals

test/cli/index/common/fail-schema-self-metaschema.sh

jviotti · 2026-03-25T17:09:14Z

test/cli/index/common/fail-self-referencing-metaschema.sh

@@ -1,12 +1,9 @@
 #!/bin/sh
-


Non-sense line removals

test/cli/index/common/fail-self-referencing-metaschema.sh

jviotti · 2026-03-25T17:09:32Z

test/cli/index/common/fail-unknown-dialect.sh

@@ -1,12 +1,9 @@
 #!/bin/sh
-


None sense line removals

test/cli/index/common/fail-unknown-dialect.sh

test/cli/index/community/fail-sourcemeta-std.sh

github-actions

Benchmark Index (enterprise)

Details

Benchmark suite	Current: `6bd83f4`	Previous: `a52e00f`	Ratio
`Add one schema (0 existing)`	`21` ms	`29` ms	`0.72`
`Add one schema (100 existing)`	`25` ms	`32` ms	`0.78`
`Add one schema (1000 existing)`	`67` ms	`92` ms	`0.73`
`Add one schema (10000 existing)`	`548` ms	`779` ms	`0.70`
`Update one schema (1 existing)`	`19` ms	`23` ms	`0.83`
`Update one schema (101 existing)`	`24` ms	`33` ms	`0.73`
`Update one schema (1001 existing)`	`66` ms	`93` ms	`0.71`
`Update one schema (10001 existing)`	`536` ms	`775` ms	`0.69`
`Cached rebuild (1 existing)`	`11` ms	`13` ms	`0.85`
`Cached rebuild (101 existing)`	`12` ms	`15` ms	`0.80`
`Cached rebuild (1001 existing)`	`25` ms	`34` ms	`0.74`
`Cached rebuild (10001 existing)`	`171` ms	`262` ms	`0.65`
`Index 100 schemas`	`112` ms	`135` ms	`0.83`
`Index 1000 schemas`	`959` ms	`1301` ms	`0.74`
`Index 10000 schemas`	`12909` ms	`16504` ms	`0.78`

This comment was automatically generated by workflow using github-action-benchmark.

github-actions

Benchmark Index (community)

Details

Benchmark suite	Current: `6bd83f4`	Previous: `a52e00f`	Ratio
`Add one schema (0 existing)`	`22` ms	`20` ms	`1.10`
`Add one schema (100 existing)`	`27` ms	`26` ms	`1.04`
`Add one schema (1000 existing)`	`83` ms	`70` ms	`1.19`
`Add one schema (10000 existing)`	`659` ms	`586` ms	`1.12`
`Update one schema (1 existing)`	`21` ms	`21` ms	`1`
`Update one schema (101 existing)`	`26` ms	`31` ms	`0.84`
`Update one schema (1001 existing)`	`87` ms	`75` ms	`1.16`
`Update one schema (10001 existing)`	`675` ms	`600` ms	`1.13`
`Cached rebuild (1 existing)`	`11` ms	`10` ms	`1.10`
`Cached rebuild (101 existing)`	`12` ms	`12` ms	`1`
`Cached rebuild (1001 existing)`	`29` ms	`26` ms	`1.12`
`Cached rebuild (10001 existing)`	`200` ms	`186` ms	`1.08`
`Index 100 schemas`	`119` ms	`121` ms	`0.98`
`Index 1000 schemas`	`996` ms	`1046` ms	`0.95`
`Index 10000 schemas`	`14725` ms	`14704` ms	`1.00`

This comment was automatically generated by workflow using github-action-benchmark.

Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>

augmentcode · 2026-03-25T17:38:32Z

🤖 Augment PR Summary

Summary: This PR improves CLI error reporting by attaching more contextual information (paths/identifiers/locations) to common failure modes during indexing and configuration parsing.

Changes:

Passes the configuration file path into Configuration::parse and surfaces it via ConfigurationValidationError::path().
Introduces OptionInvalidNumericValueError and a shared numeric-option parser to provide user-friendly diagnostics for invalid numeric flags.
Wraps several schema-related exceptions in sourcemeta::core::FileError<T> so errors can include the originating schema file path.
Extends the index CLI’s exception handling to print at path, to identifier, and JSON Pointer at location details where available.
Adds required core link dependencies (core::error, core::yaml) for the new error types/wrappers.
Updates CLI and unit tests to match the new, more descriptive error output format.

Technical Notes: The resolver now converts low-level parse/keyword/dialect issues into file-scoped errors, and indexing wraps reference/resolution failures occurring during build actions with the current schema’s path.

_{🤖 Was this summary useful? React with 👍 or 👎}

augmentcode

Review completed. 2 suggestions posted.

Comment augment review to trigger a new review at any time.

src/index/index.cc

cubic-dev-ai

1 issue found across 52 files

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/index/index.cc">

<violation number="1" location="src/index/index.cc:93">
P2: Handle `std::out_of_range` in numeric option parsing so oversized values produce the intended option-specific error message.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

src/index/index.cc

Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>

github-actions

Benchmark Index (enterprise)

Details

Benchmark suite	Current: `cf38246`	Previous: `a52e00f`	Ratio
`Add one schema (0 existing)`	`20` ms	`29` ms	`0.69`
`Add one schema (100 existing)`	`25` ms	`32` ms	`0.78`
`Add one schema (1000 existing)`	`67` ms	`92` ms	`0.73`
`Add one schema (10000 existing)`	`531` ms	`779` ms	`0.68`
`Update one schema (1 existing)`	`19` ms	`23` ms	`0.83`
`Update one schema (101 existing)`	`24` ms	`33` ms	`0.73`
`Update one schema (1001 existing)`	`66` ms	`93` ms	`0.71`
`Update one schema (10001 existing)`	`545` ms	`775` ms	`0.70`
`Cached rebuild (1 existing)`	`11` ms	`13` ms	`0.85`
`Cached rebuild (101 existing)`	`12` ms	`15` ms	`0.80`
`Cached rebuild (1001 existing)`	`25` ms	`34` ms	`0.74`
`Cached rebuild (10001 existing)`	`175` ms	`262` ms	`0.67`
`Index 100 schemas`	`118` ms	`135` ms	`0.87`
`Index 1000 schemas`	`1092` ms	`1301` ms	`0.84`
`Index 10000 schemas`	`14009` ms	`16504` ms	`0.85`

This comment was automatically generated by workflow using github-action-benchmark.

Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>

cubic-dev-ai

1 issue found across 14 files (changes from recent commits).

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/resolver/resolver.cc">

<violation number="1" location="src/resolver/resolver.cc:365">
P2: The new URI parse error path can throw again when `$id`/`id` is missing, which masks the original parse failure and produces unstable error handling.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

cubic-dev-ai · 2026-03-25T18:00:54Z

src/resolver/resolver.cc

+        sourcemeta::core::SchemaUnknownDialectError>(path);
+  } catch (const sourcemeta::core::URIParseError &) {
+    const auto reread{sourcemeta::core::read_yaml_or_json(path)};
+    const auto &id_keyword{reread.defines("$id") ? "$id" : "id"};


P2: The new URI parse error path can throw again when $id/id is missing, which masks the original parse failure and produces unstable error handling.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/resolver/resolver.cc, line 365: <comment>The new URI parse error path can throw again when `$id`/`id` is missing, which masks the original parse failure and produces unstable error handling.</comment> <file context> @@ -359,9 +360,14 @@ auto Resolver::add(const sourcemeta::core::JSON::String &server_url, - path, error.column()); + } catch (const sourcemeta::core::URIParseError &) { + const auto reread{sourcemeta::core::read_yaml_or_json(path)}; + const auto &id_keyword{reread.defines("$id") ? "$id" : "id"}; + std::ostringstream value_stream; + sourcemeta::core::stringify(reread.at(id_keyword), value_stream); </file context>

Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>

cubic-dev-ai

1 issue found across 5 files (changes from recent commits).

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/index/generators.h">

<violation number="1" location="src/index/generators.h:450">
P2: Do not rely on `assert` for `config_path` before dereferencing; this can null-dereference in release builds when the metadata key is absent.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

cubic-dev-ai · 2026-03-25T18:12:05Z

src/index/generators.h

+      assert(config_path);
+      throw CustomRuleError(std::filesystem::path{config_path->to_string()});


P2: Do not rely on assert for config_path before dereferencing; this can null-dereference in release builds when the metadata key is absent.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/index/generators.h, line 450: <comment>Do not rely on `assert` for `config_path` before dereferencing; this can null-dereference in release builds when the metadata key is absent.</comment> <file context> @@ -445,8 +445,10 @@ struct GENERATE_HEALTH { - throw CustomRuleError(); + const auto *config_path{ + configuration.extra.try_at("x-sourcemeta-one:path")}; + assert(config_path); + throw CustomRuleError(std::filesystem::path{config_path->to_string()}); } </file context>

Suggested change

assert(config_path);

throw CustomRuleError(std::filesystem::path{config_path->to_string()});

throw CustomRuleError(config_path ? std::filesystem::path{config_path->to_string()} : std::filesystem::path{});

Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>

github-actions

Benchmark Index (community)

Details

Benchmark suite	Current: `4e019a2`	Previous: `a52e00f`	Ratio
`Add one schema (0 existing)`	`21` ms	`20` ms	`1.05`
`Add one schema (100 existing)`	`25` ms	`26` ms	`0.96`
`Add one schema (1000 existing)`	`75` ms	`70` ms	`1.07`
`Add one schema (10000 existing)`	`627` ms	`586` ms	`1.07`
`Update one schema (1 existing)`	`19` ms	`21` ms	`0.90`
`Update one schema (101 existing)`	`28` ms	`31` ms	`0.90`
`Update one schema (1001 existing)`	`75` ms	`75` ms	`1`
`Update one schema (10001 existing)`	`642` ms	`600` ms	`1.07`
`Cached rebuild (1 existing)`	`11` ms	`10` ms	`1.10`
`Cached rebuild (101 existing)`	`12` ms	`12` ms	`1`
`Cached rebuild (1001 existing)`	`26` ms	`26` ms	`1`
`Cached rebuild (10001 existing)`	`198` ms	`186` ms	`1.06`
`Index 100 schemas`	`125` ms	`121` ms	`1.03`
`Index 1000 schemas`	`1009` ms	`1046` ms	`0.96`
`Index 10000 schemas`	`13702` ms	`14704` ms	`0.93`

This comment was automatically generated by workflow using github-action-benchmark.

github-actions

Benchmark Index (enterprise)

Details

Benchmark suite	Current: `4e019a2`	Previous: `a52e00f`	Ratio
`Add one schema (0 existing)`	`22` ms	`29` ms	`0.76`
`Add one schema (100 existing)`	`26` ms	`32` ms	`0.81`
`Add one schema (1000 existing)`	`76` ms	`92` ms	`0.83`
`Add one schema (10000 existing)`	`881` ms	`779` ms	`1.13`
`Update one schema (1 existing)`	`21` ms	`23` ms	`0.91`
`Update one schema (101 existing)`	`26` ms	`33` ms	`0.79`
`Update one schema (1001 existing)`	`80` ms	`93` ms	`0.86`
`Update one schema (10001 existing)`	`821` ms	`775` ms	`1.06`
`Cached rebuild (1 existing)`	`13` ms	`13` ms	`1`
`Cached rebuild (101 existing)`	`13` ms	`15` ms	`0.87`
`Cached rebuild (1001 existing)`	`29` ms	`34` ms	`0.85`
`Cached rebuild (10001 existing)`	`236` ms	`262` ms	`0.90`
`Index 100 schemas`	`120` ms	`135` ms	`0.89`
`Index 1000 schemas`	`1017` ms	`1301` ms	`0.78`
`Index 10000 schemas`	`13392` ms	`16504` ms	`0.81`

This comment was automatically generated by workflow using github-action-benchmark.

Improve error message information

6bd83f4

Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>