-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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: Don't panic for nil Encode{Time, Duration} #835
Conversation
Fixes #834 The JSON encoder assumes that encoders for `time.Time` and `time.Duration` are always specified, which causes nil pointer dereference panics. Fix this by treating nil encoders for time and duration as no-ops. This will fall back to existing logic in the JSON encoder that handles no-op time and duration encoders.
Codecov Report
@@ Coverage Diff @@
## master #835 +/- ##
=======================================
Coverage 98.33% 98.33%
=======================================
Files 43 43
Lines 2347 2349 +2
=======================================
+ Hits 2308 2310 +2
Misses 32 32
Partials 7 7
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks related to #791 where we chose to error out on creation if the time key is set, but there was no time encoder.
I think the behavioue we'll end up with is:
- If the time key is specified, a time encoder must be specified.
- If no time key is specified, and there's no time encoder, then times will be encoded with a default
Should we have the same default behaviour even when a time key is set?
Before we change the default behavior, I want to clarify that we have
We can change |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point, seems reasonable that different encoders may have different defaults.
Fixes uber-go#834 The JSON encoder assumes that encoders for `time.Time` and `time.Duration` are always specified, which causes nil pointer dereference panics. Fix this by treating nil encoders for time and duration as no-ops. This will fall back to existing logic in the JSON encoder that handles no-op time and duration encoders.
Fixes #834
The JSON encoder assumes that encoders for
time.Time
andtime.Duration
are always specified, which causes nil pointerdereference panics.
Fix this by treating nil encoders for time and duration as no-ops. This
will fall back to existing logic in the JSON encoder that handles no-op
time and duration encoders.