Include YAML node path in deserialization error messages

[lottaquestions]

Fixes dbt-labs/dbt-fusion#1613

If a YAML doc has an error, the location of the error is not being reported:

YAML error: invalid type: unit value, expected a string 

This change ensures that we include the YAML node path to indicate where the issue is:

YAML error: docs.doc.risk_data.None.name: invalid type: unit value, expected a string

[lottaquestions]

@sadboy I am not happy with the format of the error message, but I did not want to change

dbt-yaml/dbt-yaml/src/error.rs

Lines 257 to 261 in 5cf0340

	ErrorImpl::Message(msg, Some(Pos { span: _, path })) => {
	if path != "." {
	write!(f, "{}: ", path)?;
	}
	f.write_str(msg)

in case there was some other underlying reason or consistency required to have it that way.

[ChenyuLInx]

LGTM with a nit for test

[sadboy]

Thanks! Comments inlined.

[sadboy]

This is a regression no? It's harder to read with the path buried in the middle, and the double "at"s.

[lottaquestions]

It is an improvement. I have actually moved the node path closer to the end which gives better readability. Before this change, the error was actually buried in the middle with a series of colons making the readability very hard. It was not even clear that a node path was being provided. See examples below of how the message looked like after the initial change to introduce the YAML node path, but before we moved it closer to the end:

 error: dbt1501: Invalid model definition `test_as_bool.+staging`: Failed to eval the compiled Jinja expression invalid operation: cannot perform a containment check on this value
 (in dbt_project.yml:14:1)
   --> dbt_project.yml:14:17
-error: dbt1013: Invalid seed definition `another_seed.some_random_key`: invalid type: floating point `inf`, expected a string at line 22 column 15
+error: dbt1013: Invalid seed definition `another_seed.some_random_key`: seeds.__additional_properties__.another_seed.__additional_properties__.some_random_key.+group: invalid type: floating point `inf`, expected a string at line 22 column 15
   --> dbt_project.yml:22:15
-error: dbt1013: Invalid seed definition `some_seed`: invalid type: integer `111`, expected a string at line 19 column 19
+error: dbt1013: Invalid seed definition `some_seed`: seeds.__additional_properties__.some_seed.+file_format: invalid type: integer `111`, expected a string at line 19 column 19
   --> dbt_project.yml:19:19


-error: dbt1013: YAML error: data did not match any variant of untagged enum DbtPackageEntry
+error: dbt1013: YAML error: packages: data did not match any variant of untagged enum DbtPackageEntry


-error: dbt1013: Failed to parse downstream exposures file exposures/exposures_with_type_errors.json: YAML error: invalid type: string "account_id_cannot_be_string", expected i64
+error: dbt1013: Failed to parse downstream exposures file exposures/exposures_with_type_errors.json: YAML error: exposures[0]: invalid type: string "account_id_cannot_be_string", expected i64

[sadboy]

That shows the addition of the path info in error messags, which is obviously a great improvement, but irrelevant to my question -- for the issue at hand, you need to compare the error messages with the path as a prefix, vs buried in the middle with an "at".

But anyways, I get what the problem is now ;) -- essentially, fusion error messages looks bad with the default formatting of YAML errors, due to a naive rendering of dbt_yaml::Error and an overuse of colons. Yes, that much is true. However, that's not a problem for dbt_yaml -- how fusion formats its error messages is of no concern to dbt_yaml!

From the perspective of dbt_yaml, going from

"b[0].d: invalid type: string "fase", expected a boolean at line 3 column 8";
to
"invalid type: string "fase", expected a boolean at b[0].d at line 3 column 8";

is for sure a regression, so we should revert the change here. The correct fix is to address on the fusion side: when converting from a dbt_yaml::Error to an FsError, instead of doing a .to_string(), it needs to look at the structure of yaml error -- i.e. extract the path from Error -- and format accordingly. e.g. perhaps something like

-error: dbt1013: Invalid seed definition `some_seed`: seeds.some_seed.+file_format: invalid type: integer `111`, expected a string at line 19 column 19
+error: dbt1013: Invalid seed definition `some_seed`(seeds.some_seed.+file_format): invalid type: integer `111`, expected a string

Note in the above that we can also drop the at line 19 column 19, since it's duplicate with fusion's own (and much better) location rendering.

[sadboy]

Suggested change

	pub(crate) fn set_span_with_path(mut error: Error, span: Span, path: &Path) -> Error {
	pub(crate) fn set_span(mut error: Error, span: Span, path: Path) -> Error {

• Path is Copy so don't need &Path
• This should replace set_span, so we don't miss any callsites. Use Path::Root as default for places where path is not available.

[sadboy]

Excellent, thank you!