Skip to content
New issue

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

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

Already on GitHub? # to your account

1.x: replace non-serializable value of OnNextValue with its toString #4841

Merged
merged 1 commit into from
Nov 12, 2016

Conversation

akarnokd
Copy link
Member

When certain operators crash, they attach the current value to the exception via OnErrorThrowable.addValueAsLastCause, however, the value may not be serializable and the Throwable itself is, that causes a serialization exception.

This PR replaces such values with their String.valueOf representation or the crash message if toString failed on that object.

This was reported in #4562 with the suggestion of making the internal field transient. The problem with that is that upon deserialization, the field is null and there is no way of knowing if the original value was really null or just not serializable. Using String.valueOf at least gives a chance to know it was non-null and gets across some information about the original value.

@codecov-io
Copy link

codecov-io commented Nov 12, 2016

Current coverage is 84.18% (diff: 50.00%)

Merging #4841 into 1.x will increase coverage by 0.12%

@@                1.x      #4841   diff @@
==========================================
  Files           287        287          
  Lines         17856      17868    +12   
  Methods           0          0          
  Messages          0          0          
  Branches       2704       2706     +2   
==========================================
+ Hits          15011      15043    +32   
+ Misses         1973       1961    -12   
+ Partials        872        864     -8   

Powered by Codecov. Last update 2cdf1c0...c310906

@akarnokd akarnokd merged commit aa1c4ed into ReactiveX:1.x Nov 12, 2016
@akarnokd akarnokd deleted the OnErrorThrowableSerializationFix branch November 12, 2016 18:31
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants