Skip to content

fix: Unchecked error in std.manifestYamlDoc().#800

Merged
johnbartholomew merged 1 commit into
google:masterfrom
rudo-thomas:fix-manifestYamlDoc-error-check
Jun 19, 2025
Merged

fix: Unchecked error in std.manifestYamlDoc().#800
johnbartholomew merged 1 commit into
google:masterfrom
rudo-thomas:fix-manifestYamlDoc-error-check

Conversation

@rudo-thomas
Copy link
Copy Markdown
Contributor

Problem: A recursive call within manifestYamlDoc() might fail but the return value is not checked.

Reproducible with:

$ jsonnet-2f73f61 -e "std.manifestYamlDoc({ x: { y: error 'foo' } }, quote_keys=false)"
"x:\n  "

Expected:

$ jsonnet-0.20.0 -e "std.manifestYamlDoc({ x: { y: error 'foo' } }, quote_keys=false)"
RUNTIME ERROR: foo
	<cmdline>:1:31-42	object <anonymous>
	...

@rudo-thomas
Copy link
Copy Markdown
Contributor Author

@johnbartholomew, can you please get this merged? :)
Thanks.

@coveralls
Copy link
Copy Markdown

coveralls commented May 7, 2025

Coverage Status

coverage: 44.051% (-11.5%) from 55.582%
when pulling 80c7c4a on rudo-thomas:fix-manifestYamlDoc-error-check
into bfa7272 on google:master.

@rudo-thomas
Copy link
Copy Markdown
Contributor Author

Hello, how can I bring someone's attention to this issue? :-)

The fix itself is clear and obvious, but I guess I could add a test for if needed.

Cheers.

Problem: A recursive call within manifestYamlDoc() might fail but the
return value is not checked.

Reproducible with:
```
$ jsonnet-2f73f61 -e "std.manifestYamlDoc({ x: { y: error 'foo' } }, quote_keys=false)"
"x:\n  "
```

Expected:
```
$ jsonnet-0.20.0 -e "std.manifestYamlDoc({ x: { y: error 'foo' } }, quote_keys=false)"
RUNTIME ERROR: foo
	<cmdline>:1:31-42	object <anonymous>
	...
```
@johnbartholomew johnbartholomew force-pushed the fix-manifestYamlDoc-error-check branch from b957d71 to 80c7c4a Compare June 19, 2025 12:46
@johnbartholomew
Copy link
Copy Markdown
Collaborator

Sorry for the silence here, and also for missing this fix for the 0.21 release. I've rebased it to master, and will merge assuming CI checks pass.

@johnbartholomew johnbartholomew merged commit 80c7c4a into google:master Jun 19, 2025
7 checks passed
@rudo-thomas rudo-thomas deleted the fix-manifestYamlDoc-error-check branch June 19, 2025 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants