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

NopLogger: Fix nil Clock panic and release #974

Merged
merged 2 commits into from
Jun 28, 2021
Merged

NopLogger: Fix nil Clock panic and release #974

merged 2 commits into from
Jun 28, 2021

Conversation

abhinav
Copy link
Collaborator

@abhinav abhinav commented Jun 28, 2021

In #897, we added a zap.Clock option to control the source of time
but neglected to set this field on the logger constructed by
zap.NewNop. This has the effect of panicking the Nop logger with a nil
dereference.

Fix the nil dereference and add checks for the behavior of the Nop
logger, and tag a new patch release with the fix.

Verified that these are the only instantiations of Logger in this
package:

$ rg '\bLogger\{' *.go
logger_test.go
67:                     for _, logger := range []*Logger{grandparent, parent, child} {

logger.go
71:     log := &Logger{
86:     return &Logger{

Refs GO-684

In #897, we added a `zap.Clock` option to control the source of time
but neglected to set this field on the logger constructed by
`zap.NewNop`. This has the effect of panicking the Nop logger with a nil
dereference.

Fix the nil dereference and add checks for the behavior of the Nop
logger.

Verified that these are the only instantiations of `Logger` in this
package:

```
$ rg '\bLogger\{' *.go
logger_test.go
67:                     for _, logger := range []*Logger{grandparent, parent, child} {

logger.go
71:     log := &Logger{
86:     return &Logger{
```

Refs GO-684
@codecov
Copy link

codecov bot commented Jun 28, 2021

Codecov Report

Merging #974 (a779980) into master (35f15d1) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #974   +/-   ##
=======================================
  Coverage   98.19%   98.20%           
=======================================
  Files          46       46           
  Lines        2055     2056    +1     
=======================================
+ Hits         2018     2019    +1     
  Misses         29       29           
  Partials        8        8           
Impacted Files Coverage Δ
logger.go 96.22% <100.00%> (+0.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 35f15d1...a779980. Read the comment docs.

This tags a patch release of the breakage introduced in 1.18.0.
@abhinav abhinav changed the title NopLogger: Fix nil Clock panic NopLogger: Fix nil Clock panic and release Jun 28, 2021
@abhinav abhinav requested a review from prashantv June 28, 2021 19:05
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants