Skip to content
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

json resource: ensure params is not nil in event of read/parse failure #2354

Merged
merged 1 commit into from
Nov 29, 2017

Conversation

adamleff
Copy link
Contributor

When the JSON resource (and those that subclass off of it) were modified to properly throw exceptions in the event of failure, this caused the params method to return nil instead of what it used to be, an empty hash.

This is fine in the case of a describe block, but it's not okay when used outside of a describe, as it will cause users trying to pluck from the hash to throw a dreaded-and-unhelpful NilClass error.

This change pre-populates the params to be an empty hash, and if the read/parse steps fail, it will still be one.

Fixes #2351

When the JSON resource (and those that subclass off of it) were modified
to properly throw exceptions in the event of failure, this caused the
`params` method to return nil instead of what it used to be, an empty
hash.

This is fine in the case of a describe block, but it's not okay when used
outside of a describe, as it will cause users trying to pluck from the
hash to throw a dreaded-and-unhelpful NilClass error.

This change pre-populates the params to be an empty hash, and if the
read/parse steps fail, it will still be one.

Signed-off-by: Adam Leff <[email protected]>
@adamleff adamleff requested a review from a team as a code owner November 29, 2017 21:12
@adamleff adamleff added the Type: Bug Feature not working as expected label Nov 29, 2017
@adamleff adamleff changed the title json resource: ensure params is not nil in even of read/parse failure json resource: ensure params is not nil in event of read/parse failure Nov 29, 2017
Copy link
Contributor

@alexpop alexpop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified, thank you Adam for taking care of this!

@adamleff adamleff requested a review from chris-rock November 29, 2017 21:18
Copy link
Contributor

@chris-rock chris-rock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @adamleff for this quick fix

@adamleff adamleff merged commit 12fec23 into master Nov 29, 2017
@adamleff adamleff deleted the adamleff/json-empty-params branch November 29, 2017 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Feature not working as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants