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

Unserializing YAML attributes can silently fail in development mode #9975

Merged

Conversation

mmangino
Copy link
Contributor

If you use a serialized attribute on an ActiveRecord model and the object to be unserialized has not yet been loaded, AR will silently return the YAML string. This can be confusing to new users and apparently to me, even though I've been bitten by this three or four times in the past.

@mmangino
Copy link
Contributor Author

The above patch will at least re-raise the argument error. I'm not convinced we should ever swallow these exceptions though. When would we want to get a string of bad YAML back instead of being told our object is invalid? That seems surprising.

If we want to keep swallowing some errors, it might be nice to create another custom error class that gives the missing class name and suggests that the user require_dependency it. I'm happy to do that if there is interest.

@egilburg
Copy link
Contributor

Is this really an ArgumentError? Seems more like TypeError or similar

@mmangino
Copy link
Contributor Author

The error comes from YAML, so I can't control that. We could wrap it, I'm just not sure if it makes sense.

@mmangino
Copy link
Contributor Author

mmangino commented Apr 2, 2013

@tenderlove This has been updated as we discussed.

tenderlove added a commit that referenced this pull request Apr 2, 2013
…unserialized

Unserializing YAML attributes can silently fail in development mode
@tenderlove tenderlove merged commit 22fee7c into rails:master Apr 2, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants