Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

rudo-thomas
Copy link
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
Contributor Author

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

@coveralls
Copy link

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
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
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