-
Notifications
You must be signed in to change notification settings - Fork 44
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
Panic reduction #615
Closed
11 tasks done
Labels
bug
Something isn't working
Comments
Defering panics in client/document as the correct behaviour requires a public function signature change (to return the error), and IMO that is not suitable for a patch release (v0.x.y or not). |
1 task
1 task
Moved to v0.4.0 as remaining items require pub signature changes |
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
A tracking issue for minimizing panic.
The risk with
panic()
is that it leads to runtime crash.Acceptable kinds of panic:
recover()
Paths forward:
Here are the current points calling
panic()
(as of commit 16084c8 ...), excluding tests.I'm marking each as a task that can be marked off with a decision/change.
defradb/client/document.go
Line 397 in 16084c8
defradb/client/document.go
Line 402 in 16084c8
defradb/db/db.go
Line 224 in 16084c8
defradb/db/container/container.go
Line 43 in 16084c8
defradb/db/container/container.go
Line 67 in 16084c8
defradb/net/utils/util.go
Line 31 in 16084c8
defradb/core/data.go
Line 54 in 16084c8
defradb/core/data.go
Line 59 in 16084c8
defradb/logging/logger.go
Line 35 in 16084c8
must
)defradb/core/net/protocol.go
Line 42 in 16084c8
defradb/merkle/crdt/composite.go
Line 51 in 16084c8
defradb/merkle/crdt/lwwreg.go
Line 47 in 16084c8
The text was updated successfully, but these errors were encountered: