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

Encode errors under different keys #93

Merged
merged 1 commit into from
Jul 6, 2016
Merged

Encode errors under different keys #93

merged 1 commit into from
Jul 6, 2016

Conversation

akshayjshah
Copy link
Contributor

Before opening your pull request, please make sure that you've:

Currently, we don't handle encoding errors in a consistent way - AddObject serializes the error into the log message, but AddMarshaler logs the message to standard out. Even though very few portions of the code can fail at runtime, we end up checking and managing errors all over the place.

This PR allows the encoder to return an error for AddObject and AddMarshaler, which are the only two methods that can fail. However, the field handles that error by serializing it under a different key
(errors for the key foo are serialized under fooError).

This should make our APIs less cumbersome and our error handling more consistent. It fixes #43.

@akshayjshah
Copy link
Contributor Author

Force-pushed after resolving merge conflict.

@prashantv
Copy link
Collaborator

lgtm

Currently, we don't handle encoding errors in a consistent way
- `AddObject` serializes the error into the log message, but
`AddMarshaler` logs the message to standard out. Even though very few
portions of the code can fail at runtime, we end up checking and
managing errors all over the place.

This PR allows the encoder to return an error for `AddObject` and
`AddMarshaler`, which are the only two methods that can fail. However,
the field handles that error by serializing it under a different key
(errors for the key `foo` are serialized under `fooError`).

This should make our APIs less cumbersome and our error handling more
consistent. It fixes #43.
@akshayjshah
Copy link
Contributor Author

Resolved more merge conflicts, force-pushed. Will merge once tests pass.

@akshayjshah akshayjshah merged commit 63d56df into master Jul 6, 2016
@akshayjshah akshayjshah deleted the ajs-errs branch July 26, 2016 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Encode errors under different keys
2 participants