feat: harden config error messages in loading paths#117
Merged
Conversation
…re absent
Missing required fields in node definitions silently produce nil values
that the evaluator later dereferences, giving a confusing crash at
runtime instead of a helpful message at load time.
Now `parse_concept_fields` calls `warn_missing_node_fields/2` after
every node parse, emitting a Logger.warning that names the node
({type_id, concept_id, field_name}) and the missing field:
- :generated nodes must have "method"; without it the evaluator cannot
find a rolling method to use.
- :mapping nodes must have "input" and "steps"; without either, the
mapping lookup cannot run.
- :accumulator and :formula nodes have no required optional fields
beyond their structural keys, so they are not validated here.
Similarly, rolling methods missing "dice" now warn with the method id;
without dice the expression parser will crash on the first roll.
Logger.warning rather than error return, consistent with the metadata
key validation added previously. A future breaking release can harden
these to errors.
…nd metadata_contribution
A [[concept_type]] entry without an id, or a [[metadata_contributions]]
entry without from_type/from_field/to_type/to_field/value, previously
produced a struct with nil fields that silently misbehaved: a nil id
would be included in the concept_type_ids MapSet, and a nil from_type
would cause the contribution to match no concepts and generate no
effects, with no indication of the problem.
from_map/1 now validates both lists before building the struct:
- validate_concept_types/1: {:error, {:concept_type_missing_id, idx}}
if any entry lacks an id field.
- validate_metadata_contributions/1: {:error,
{:metadata_contribution_missing_field, field, idx}} if any entry is
missing one of the five required fields.
These return {:error, ...} from from_map/1 (and therefore from
Loader.load/1) rather than Logger.warning, because a metadata_contribution
with a nil from_type or a concept_type with a nil id cannot be recovered
from at runtime — the data is structurally invalid.
Restructured from_map/1 to use a top-level with/else rather than
if/else + nested with, eliminating a Credo nesting violation. Extracted
check_metadata_contribution_fields/1 to flatten the validate function
for the same reason.
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
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.
Summary
feat(loader)): Addswarn_missing_node_fields/2called after every node parse. Emits aLogger.warningnaming the node{type_id, concept_id, field_name}when required fields are absent::generatednodes missing"method"— the evaluator cannot find a rolling method:mappingnodes missing"input"or"steps"— the mapping lookup cannot run"dice"— the dice parser crashes on first rollfeat(rule_module)):from_map/1now validates concept type and metadata contribution lists before building the struct:[[concept_type]]entry missingid→{:error, {:concept_type_missing_id, idx}}[[metadata_contributions]]entry missing any offrom_type/from_field/to_type/to_field/value→{:error, {:metadata_contribution_missing_field, field, idx}}from_map/1fromif/else + withto a flatwith/elseto fix a Credo nesting violationPreviously all of these failed silently (nil values, no effects generated, or runtime evaluator crash with no load-time signal).
Test plan
Summary by Bito