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.

Sign up for GitHub

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

Jump to bottom

Don't Panic in the Server #497

Closed
hackerwins opened this issue Mar 22, 2023 · 11 comments · Fixed by #636
Closed

Don't Panic in the Server #497

hackerwins opened this issue Mar 22, 2023 · 11 comments · Fixed by #636
Labels
cleanup 🧹 Paying off technical debt good first issue 🐤 Good for newcomers

Comments

@hackerwins
Copy link
Member

Description:

We often use panic in development to reveal errors, but codes running in production must avoid panic to prevent stopping the server.

Refer to https://github.com/uber-go/guide/blob/master/style.md#dont-panic

Why:

  • Prevent stopping the server.
@hackerwins hackerwins added good first issue 🐤 Good for newcomers cleanup 🧹 Paying off technical debt labels Mar 22, 2023
@emplam27 emplam27 mentioned this issue Mar 24, 2023
Closed
2 tasks
@hackerwins
Copy link
Member Author

Some panic in the logic running on the server was fixed by #524, #522, #519.

@hackerwins
Copy link
Member Author

The logic of crdt package is executed when creating a snapshot on the server, whereas the logic of the json package is only executed when users use the SDK, not on the server. Additionally, removing panic and adding errors as method responses can make the interface somewhat messy.

Therefore, we have decided to replace panic with errors in the crdt package and keep panic in json package.

@hackerwins
Copy link
Member Author

The list of panics that need to be replaced with err from current main branch is as follows:

$ grep panic pkg -R
pkg/document/crdt/element_rht.go:		panic("fail to find: " + elem.CreatedAt().Key())
pkg/document/crdt/tree.go:		panic(fmt.Errorf("cannot find node at %p", pos))
pkg/document/crdt/tree.go:		panic(fmt.Errorf("cannot find node at %p", pos))
pkg/document/crdt/tree.go:			panic(errors.New("left and right are not in the same list"))
pkg/document/crdt/rga_tree_list.go:		panic("fail to find the given createdAt: " + createdAt.Key())
pkg/document/crdt/rga_tree_list.go:		panic("fail to find the given prevCreatedAt: " + prevCreatedAt.Key())
pkg/document/crdt/rga_tree_list.go:		panic("fail to find the given createdAt: " + createdAt.Key())
pkg/document/crdt/rga_tree_list.go:		panic("fail to find the given prevCreatedAt: " + createdAt.Key())
pkg/document/crdt/rga_tree_list.go:		panic("fail to find the given createdAt: " + elem.CreatedAt().Key())
pkg/document/crdt/rga_tree_list.go:		panic("fail to find the given createdAt: " + createdAt.Key())
pkg/document/crdt/rga_tree_split.go:		panic("RGATreeSplitNodeID cannot be null")
pkg/document/crdt/primitive.go:	panic("unsupported type")
pkg/document/crdt/primitive.go:	panic("unsupported type")
pkg/document/crdt/primitive.go:	panic("unsupported type")
pkg/document/crdt/primitive.go:	panic("unsupported type")
pkg/document/crdt/counter.go:	panic("unsupported type")
pkg/document/crdt/counter.go:	panic("unsupported type")
pkg/document/crdt/counter.go:	panic("unsupported type")
pkg/document/crdt/counter.go:		panic("unsupported type")
pkg/document/crdt/counter.go:		panic("unsupported type")
pkg/document/crdt/counter.go:		panic("unsupported type")
pkg/document/crdt/counter.go:		panic("unsupported type")
pkg/document/document.go:		panic(err)
pkg/index/tree.go:		panic(fmt.Sprintf("from cannot be greater than to %d > %d", from, to))
pkg/index/tree.go:		panic(fmt.Sprintf("from is out of range %d > %d", from, root.Length))
pkg/index/tree.go:		panic(fmt.Sprintf("to is out of range %d > %d", to, root.Length))
pkg/index/tree.go:		panic(errors.New("text node cannot have children"))
pkg/index/tree.go:		panic(errors.New("text node cannot have children"))
pkg/index/tree.go:		panic(errors.New("text node cannot have children"))
pkg/index/tree.go:		panic(errors.New("text node cannot have children"))
pkg/index/tree.go:		panic(errors.New("prevNode is not a child of the node"))
pkg/index/tree.go:		panic(errors.New("text node cannot have children"))
pkg/index/tree.go:		panic(errors.New("text node cannot have children"))
pkg/index/tree.go:		panic(errors.New("text node cannot have children"))
pkg/index/tree.go:		panic(errors.New("text node cannot have children"))
pkg/index/tree.go:		panic(errors.New("text node cannot have children"))
pkg/index/tree.go:		panic(errors.New("text node cannot have children"))
pkg/index/tree.go:		panic(errors.New("child not found"))
pkg/index/tree.go:		panic(errors.New("text node cannot have children"))
pkg/index/tree.go:		panic(errors.New("child not found"))
pkg/index/tree.go:		panic(fmt.Errorf("index is out of range: %d > %d", index, node.Length))
pkg/index/tree.go:			panic("invalid treePos")
pkg/index/tree.go:			panic("invalid treePos")
pkg/index/tree.go:		panic("unacceptable path")
pkg/index/tree.go:			panic("unacceptable path")
pkg/index/tree.go:		panic("unacceptable path")
pkg/index/tree.go:		panic("unacceptable path")
pkg/index/tree.go:			panic(errors.New("parent is not found"))

@JOOHOJANG
Copy link
Contributor

JOOHOJANG commented Jul 3, 2023 •
edited by hackerwins
Loading

some tasks have been done from #570
but, there're still few tasks remaining

I'll leave checkboxes for remaining tasks

  • crdt/rga_tree_list
  • crdt/rga_tree_split
  • crdt/primitive
  • crdt/counter
pkg/document/crdt/counter_test.go:		// when 'Remove panic from server code (#50)' is completed.
pkg/document/crdt/rga_tree_list.go:		panic("fail to find the given createdAt: " + createdAt.Key())
pkg/document/crdt/rga_tree_list.go:		panic("fail to find the given prevCreatedAt: " + prevCreatedAt.Key())
pkg/document/crdt/rga_tree_list.go:		panic("fail to find the given createdAt: " + createdAt.Key())
pkg/document/crdt/rga_tree_list.go:		panic("fail to find the given prevCreatedAt: " + createdAt.Key())
pkg/document/crdt/rga_tree_list.go:		panic("fail to find the given createdAt: " + createdAt.Key())
pkg/document/crdt/rga_tree_split.go:// argument is nil, it would panic at runtime. This method is following
pkg/document/crdt/rga_tree_split.go:		panic("RGATreeSplitNodeID cannot be null")
pkg/document/crdt/primitive.go:	panic("unsupported type")
pkg/document/crdt/primitive.go:	panic("unsupported type")
pkg/document/crdt/primitive.go:	panic("unsupported type")
pkg/document/crdt/primitive.go:	panic("unsupported type")
pkg/document/crdt/counter.go:	panic("unsupported type")
pkg/document/crdt/counter.go:	panic("unsupported type")
pkg/document/crdt/counter.go:	panic("unsupported type")
pkg/document/crdt/counter.go:		panic("unsupported type")
pkg/document/crdt/counter.go:		panic("unsupported type")
pkg/document/crdt/counter.go:		panic("unsupported type")
pkg/document/crdt/counter.go:		panic("unsupported type")
pkg/document/crdt/rga_tree_split_test.go:	t.Run("compare nil id panic test", func(t *testing.T) {
pkg/document/document_test.go:		// when 'Remove panic from server code (#50)' is completed.
pkg/document/time/ticket.go:// If the receiver or argument is nil, it would panic at runtime.
pkg/document/time/actor_id.go:// If the receiver or argument is nil, it would panic at runtime.
pkg/document/json/object.go:			panic("unsupported type")
pkg/document/json/object.go:		panic("unsupported type")
pkg/document/json/object.go:		panic("unsupported type")
pkg/document/json/object.go:		panic("unsupported type")
pkg/document/json/object.go:		panic("unsupported type")
pkg/document/json/object.go:		panic("unsupported type")
pkg/document/json/object.go:		panic(err)
pkg/document/json/tree.go:		panic("from should be less than or equal to to")
pkg/document/json/tree.go:		panic(fromErr)
pkg/document/json/tree.go:		panic(toErr)
pkg/document/json/tree.go:		panic(err)
pkg/document/json/tree.go:		panic(fromErr)
pkg/document/json/tree.go:		panic(toErr)
pkg/document/json/tree.go:		panic(err)
pkg/document/json/tree.go:		panic("from should be less than or equal to to")
pkg/document/json/tree.go:		panic(fromErr)
pkg/document/json/tree.go:		panic(toErr)
pkg/document/json/tree.go:		panic(err)
pkg/document/json/text.go:		panic(err)
pkg/document/json/text.go:		panic("from should be less than or equal to to")
pkg/document/json/text.go:		panic(err)
pkg/document/json/text.go:		panic(err)
pkg/document/json/text.go:		panic("from should be less than or equal to to")
pkg/document/json/text.go:		panic(err)
pkg/document/json/text.go:		panic(err)
pkg/document/json/text.go:		panic("from should be less than or equal to to")
pkg/document/json/text.go:		panic(err)
pkg/document/json/counter.go:		panic("unsupported type")
pkg/document/json/counter.go:		panic("unsupported type")
pkg/document/json/counter.go:		panic("unsupported type")
pkg/document/json/json.go:	panic("unsupported type")
pkg/document/json/array.go:		panic(err)
pkg/document/json/array.go:		panic(err)
pkg/document/json/array.go:		panic(err)
pkg/document/document.go:		panic(err)
pkg/locker/locker_test.go:			// if there is a concurrency issue, will very likely panic here

@hackerwins hackerwins mentioned this issue Jul 3, 2023
Merged
2 tasks
@sejongk
Copy link
Contributor

sejongk commented Aug 1, 2023

May I try this for crdt/rga_tree_list?

@hackerwins
Copy link
Member Author

@sejongk Give it a try. 💪

@MoonGyu1
Copy link
Contributor

MoonGyu1 commented Aug 2, 2023

May I work on crdt/counter of this issue?

@sejongk sejongk mentioned this issue Aug 2, 2023
Merged
2 tasks
@hackerwins
Copy link
Member Author

@MoonGyu1 Sure. If you have any questions, please let me know.

@MoonGyu1 MoonGyu1 mentioned this issue Aug 4, 2023
Merged
2 tasks
@hackerwins
Copy link
Member Author

Only the primitive remains:

  • crdt/primitive
$ grep panic pkg -R                                                                                           
pkg/document/crdt/rga_tree_split.go:// argument is nil, it would panic at runtime. This method is following
pkg/document/crdt/rga_tree_split.go:		panic("RGATreeSplitNodeID cannot be null")
pkg/document/crdt/primitive.go:	panic("unsupported type")
pkg/document/crdt/primitive.go:	panic("unsupported type")
pkg/document/crdt/primitive.go:	panic("unsupported type")
pkg/document/crdt/primitive.go:	panic("unsupported type")
pkg/document/crdt/rga_tree_split_test.go:	t.Run("compare nil id panic test", func(t *testing.T) {
pkg/document/document_test.go:		// when 'Remove panic from server code (#50)' is completed.
pkg/document/time/ticket.go:// If the receiver or argument is nil, it would panic at runtime.
pkg/document/time/actor_id.go:// If the receiver or argument is nil, it would panic at runtime.
pkg/document/json/object.go:				panic(err)
pkg/document/json/object.go:				panic(err)
pkg/document/json/object.go:			panic("unsupported type")
pkg/document/json/object.go:		panic("unsupported type")
pkg/document/json/object.go:		panic("unsupported type")
pkg/document/json/object.go:		panic("unsupported type")
pkg/document/json/object.go:		panic("unsupported type")
pkg/document/json/object.go:		panic("unsupported type")
pkg/document/json/object.go:		panic(err)
pkg/document/json/tree.go:		panic(ErrIndexBoundary)
pkg/document/json/tree.go:		panic(err)
pkg/document/json/tree.go:		panic(err)
pkg/document/json/tree.go:			panic(err)
pkg/document/json/tree.go:						panic(err)
pkg/document/json/tree.go:				panic(err)
pkg/document/json/tree.go:		panic(err)
pkg/document/json/tree.go:		panic(ErrPathLenDiff)
pkg/document/json/tree.go:		panic(ErrEmptyPath)
pkg/document/json/tree.go:		panic(err)
pkg/document/json/tree.go:		panic(err)
pkg/document/json/tree.go:		panic("from should be less than or equal to to")
pkg/document/json/tree.go:		panic(err)
pkg/document/json/tree.go:		panic(err)
pkg/document/json/tree.go:		panic(err)
pkg/document/json/tree.go:			panic(err)
pkg/document/json/text.go:		panic(err)
pkg/document/json/text.go:		panic("from should be less than or equal to to")
pkg/document/json/text.go:		panic(err)
pkg/document/json/text.go:		panic(err)
pkg/document/json/text.go:		panic("from should be less than or equal to to")
pkg/document/json/text.go:		panic(err)
pkg/document/json/text.go:		panic(err)
pkg/document/json/counter.go:		panic("unsupported type")
pkg/document/json/counter.go:		panic("unsupported type")
pkg/document/json/counter.go:		panic("unsupported type")
pkg/document/json/counter.go:		panic(err)
pkg/document/json/json.go:	panic("unsupported type")
pkg/document/json/array.go:		panic(err)
pkg/document/json/array.go:		panic(err)
pkg/document/json/array.go:		panic(err)
pkg/document/json/array.go:		panic(err)
pkg/document/json/array.go:		panic(err)
pkg/document/json/array.go:		panic(err)
pkg/document/document.go:		panic(err)
pkg/document/document.go:			panic(err)
pkg/document/document.go:		panic(err)
pkg/locker/locker_test.go:			// if there is a concurrency issue, will very likely panic here

@fourjae
Copy link
Contributor

fourjae commented Aug 31, 2023

May I try this for crdt/primitive?

@krapie
Copy link
Member

krapie commented Aug 31, 2023

@fourjae Sure!

@fourjae fourjae mentioned this issue Sep 5, 2023
Merged
2 tasks
@github-project-automation github-project-automation bot moved this from Todo to Done in Yorkie Project Sep 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
cleanup 🧹 Paying off technical debt good first issue 🐤 Good for newcomers
Projects
No open projects
Yorkie Project
Status: Done
Milestone
No milestone
6 participants
@hackerwins @sejongk @JOOHOJANG @fourjae @krapie @MoonGyu1