Got 'panic: offset should be less than or equal to length' when inserting emoji #110

habibrosyad · 2020-12-01T13:39:13Z

What happened:
I got a panic error in the agent when trying to insert emoji in the Quill example.

What you expected to happen:
No error occurred.

How to reproduce it (as minimally and precisely as possible):
Try to insert/copy-paste an emoji, then repeat it again 1 more time.

Anything else we need to know?:
Below are the logs snippet:

Attaching to docker_yorkie_1
yorkie_1  | 2020-12-01T13:22:53.728Z	INFO	connected, URI: mongodb://mongo:27017, DB: yorkie-meta
yorkie_1  | 2020-12-01T13:22:53.729Z	INFO	serving API on 9090
yorkie_1  | 2020-12-01T13:23:07.182Z	INFO	stream "/api.Yorkie/WatchDocuments" => ok
yorkie_1  | 2020-12-01T13:23:07.347Z	INFO	RPC : "/api.Yorkie/ActivateClient" 3.6611ms
yorkie_1  | 2020-12-01T13:23:07.385Z	INFO	RPC : "/api.Yorkie/AttachDocument" 13.0884ms
yorkie_1  | 2020-12-01T13:23:07.436Z	INFO	PUSH: '5fc643bb3e80833e393fe425' pushes 1 changes into 'examples$quill', rejected 0 changes, serverSeq: 0 -> 1, cp: serverSeq=1, clientSeq=1
yorkie_1  | 2020-12-01T13:23:07.442Z	INFO	RPC : "/api.Yorkie/PushPull" 12.6078ms
yorkie_1  | 2020-12-01T13:23:07.449Z	INFO	SNAP: 'examples$quill', serverSeq:1 6.3741ms
yorkie_1  | 2020-12-01T13:23:34.218Z	INFO	PUSH: '5fc643bb3e80833e393fe425' pushes 1 changes into 'examples$quill', rejected 0 changes, serverSeq: 1 -> 2, cp: serverSeq=2, clientSeq=2
yorkie_1  | 2020-12-01T13:23:34.229Z	INFO	RPC : "/api.Yorkie/PushPull" 15.6009ms
yorkie_1  | 2020-12-01T13:23:34.241Z	INFO	SNAP: 'examples$quill', serverSeq:2 11.547ms
yorkie_1  | 2020-12-01T13:23:36.263Z	INFO	PUSH: '5fc643bb3e80833e393fe425' pushes 1 changes into 'examples$quill', rejected 0 changes, serverSeq: 2 -> 3, cp: serverSeq=3, clientSeq=3
yorkie_1  | 2020-12-01T13:23:36.271Z	INFO	RPC : "/api.Yorkie/PushPull" 11.3318ms
yorkie_1  | 2020-12-01T13:23:36.277Z	INFO	SNAP: 'examples$quill', serverSeq:3 5.9041ms
yorkie_1  | 2020-12-01T13:23:37.293Z	INFO	PUSH: '5fc643bb3e80833e393fe425' pushes 1 changes into 'examples$quill', rejected 0 changes, serverSeq: 3 -> 4, cp: serverSeq=4, clientSeq=4
yorkie_1  | 2020-12-01T13:23:37.300Z	INFO	RPC : "/api.Yorkie/PushPull" 11.0282ms
yorkie_1  | 2020-12-01T13:23:37.305Z	ERROR	[0:0:00:0 {} ""][3:1:25:0 {} "🎁"][1:1:25:0 {} "
yorkie_1  | "]
yorkie_1  | github.com/yorkie-team/yorkie/pkg/document/json.(*RGATreeSplit).splitNode
yorkie_1  | 	/app/pkg/document/json/rga_tree_split.go:299
yorkie_1  | github.com/yorkie-team/yorkie/pkg/document/json.(*RGATreeSplit).findNodeWithSplit
yorkie_1  | 	/app/pkg/document/json/rga_tree_split.go:270
yorkie_1  | github.com/yorkie-team/yorkie/pkg/document/json.(*RGATreeSplit).edit
yorkie_1  | 	/app/pkg/document/json/rga_tree_split.go:371
yorkie_1  | github.com/yorkie-team/yorkie/pkg/document/json.(*RichText).Edit
yorkie_1  | 	/app/pkg/document/json/rich_text.go:191
yorkie_1  | github.com/yorkie-team/yorkie/pkg/document/operation.(*RichEdit).Execute
yorkie_1  | 	/app/pkg/document/operation/rich_edit.go:59
yorkie_1  | github.com/yorkie-team/yorkie/pkg/document/change.(*Change).Execute
yorkie_1  | 	/app/pkg/document/change/change.go:48
yorkie_1  | github.com/yorkie-team/yorkie/pkg/document.(*InternalDocument).applyChanges
yorkie_1  | 	/app/pkg/document/internal_document.go:204
yorkie_1  | github.com/yorkie-team/yorkie/pkg/document.(*InternalDocument).ApplyChangePack
yorkie_1  | 	/app/pkg/document/internal_document.go:108
yorkie_1  | github.com/yorkie-team/yorkie/yorkie/packs.storeSnapshot
yorkie_1  | 	/app/yorkie/packs/pack_service.go:367
yorkie_1  | github.com/yorkie-team/yorkie/yorkie/packs.PushPull.func1
yorkie_1  | 	/app/yorkie/packs/pack_service.go:119
yorkie_1  | github.com/yorkie-team/yorkie/yorkie/backend.(*Backend).AttachGoroutine.func1
yorkie_1  | 	/app/yorkie/backend/backend.go:101
yorkie_1  | panic: offset should be less than or equal to length
yorkie_1  | 
yorkie_1  | goroutine 56 [running]:
yorkie_1  | github.com/yorkie-team/yorkie/pkg/document/json.(*RGATreeSplit).splitNode(0xc000442240, 0xc00042eaa0, 0x2, 0x20)
yorkie_1  | 	/app/pkg/document/json/rga_tree_split.go:300 +0x265
yorkie_1  | github.com/yorkie-team/yorkie/pkg/document/json.(*RGATreeSplit).findNodeWithSplit(0xc000442240, 0xc000424c10, 0xc0004421c0, 0x40f1b0, 0x140e800)
yorkie_1  | 	/app/pkg/document/json/rga_tree_split.go:270 +0xc5
yorkie_1  | github.com/yorkie-team/yorkie/pkg/document/json.(*RGATreeSplit).edit(0xc000442240, 0xc000424bf0, 0xc000424c10, 0xc000423d70, 0xf3d9a0, 0xc0004423e0, 0xc0004421c0, 0xc00008ac30, 0xc000400700)
yorkie_1  | 	/app/pkg/document/json/rga_tree_split.go:371 +0x5a
yorkie_1  | github.com/yorkie-team/yorkie/pkg/document/json.(*RichText).Edit(0xc0004440c0, 0xc000424bf0, 0xc000424c10, 0xc000423d70, 0xc00041988c, 0x4, 0x0, 0xc0004421c0, 0x13bef28, 0xc6fa20)
yorkie_1  | 	/app/pkg/document/json/rich_text.go:191 +0x205
yorkie_1  | github.com/yorkie-team/yorkie/pkg/document/operation.(*RichEdit).Execute(0xc0004306c0, 0xc0004423a0, 0xc0004440f0, 0xc00041f380)
yorkie_1  | 	/app/pkg/document/operation/rich_edit.go:59 +0xd8
yorkie_1  | github.com/yorkie-team/yorkie/pkg/document/change.(*Change).Execute(0xc000430700, 0xc0004423a0, 0xcee780, 0xb7decc)
yorkie_1  | 	/app/pkg/document/change/change.go:48 +0x70
yorkie_1  | github.com/yorkie-team/yorkie/pkg/document.(*InternalDocument).applyChanges(0xc000430940, 0xc000010bd8, 0x1, 0x1, 0x7f130c5fba30, 0x0)
yorkie_1  | 	/app/pkg/document/internal_document.go:204 +0x66
yorkie_1  | github.com/yorkie-team/yorkie/pkg/document.(*InternalDocument).ApplyChangePack(0xc000430940, 0xc00026fdf0, 0xc000418ec9, 0x5)
yorkie_1  | 	/app/pkg/document/internal_document.go:108 +0x325
yorkie_1  | github.com/yorkie-team/yorkie/yorkie/packs.storeSnapshot(0xf3b420, 0xc00003a018, 0xc0002a48c0, 0xc0000d5280, 0x0, 0xc000378500)
yorkie_1  | 	/app/yorkie/packs/pack_service.go:367 +0x32e
yorkie_1  | github.com/yorkie-team/yorkie/yorkie/packs.PushPull.func1()
yorkie_1  | 	/app/yorkie/packs/pack_service.go:119 +0x3d1
yorkie_1  | github.com/yorkie-team/yorkie/yorkie/backend.(*Backend).AttachGoroutine.func1(0xc0002a48c0, 0xc0004239e0)
yorkie_1  | 	/app/yorkie/backend/backend.go:101 +0x55
yorkie_1  | created by github.com/yorkie-team/yorkie/yorkie/backend.(*Backend).AttachGoroutine
yorkie_1  | 	/app/yorkie/backend/backend.go:99 +0x157

Environment:

Operating system: macOS
Browser and version: Google Chrome 87.0.4280.67
Yorkie version (use yorkie version): master branch
Yorkie JS SDK version: master branch

The text was updated successfully, but these errors were encountered:

hackerwins · 2020-12-02T00:43:28Z

Thank you for your report. It seems to be an issue related to the length of the emoji.

habibrosyad · 2020-12-02T01:38:13Z

Yes, that's the issue. In Golang for some unicode characters, if you count character by len, the result might not be as expected. This post in SO might be useful as a reference.

Just found out that an emoji length in JS is commonly 2, while in Go it is 4. But yeah, not always, it varies, e.g. "🇦🇬".length = 4, "🏴󠁵󠁳󠁴󠁸󠁿".length = 12.

habibrosyad · 2020-12-02T15:06:30Z

I'm not fully grasped the side effect yet as I'm still exploring the codebase. It seems, changing

yorkie/pkg/document/json/rga_tree_split.go

Lines 298 to 301 in efd0e5f

    
           if offset > node.contentLen() { 
        
           	log.Logger.Error(s.AnnotatedString()) 
        
           	panic("offset should be less than or equal to length") 
        
           }

into

if offset > node.contentLen() {
	// Instead of panicking let's just assume offset = content len here.
	offset = node.contentLen()
}

will solve the issue (tested via the Quill example). @hackerwins do you think this will popup another issue?

hackerwins · 2020-12-03T00:38:35Z

Thank you for thinking about the solution.

But the suggestion could bypass the panic, but it doesn't seem to fix the underlying cause.

I think the problem will be solved if Split and Len of RichText(and Text) treat emoji as one character(
currently, only utf-8 characters are considered). 🤔

yorkie/pkg/document/json/rich_text.go

Lines 67 to 72 in efd0e5f

    
           func (t *RichTextValue) Split(offset int) RGATreeSplitValue { 
        
           	value := t.value 
        
           	r := []rune(value) 
        
           	t.value = string(r[0:offset]) 
        
           	return NewRichTextValue(t.attrs.DeepCopy(), string(r[offset:])) 
        
           }

yorkie/pkg/document/json/rich_text.go

Lines 55 to 57 in efd0e5f

    
           func (t *RichTextValue) Len() int { 
        
           	return utf8.RuneCountInString(t.value) 
        
           }

In JS SDK

hackerwins · 2021-03-27T03:24:07Z

@habibrosyad Thanks to you, we fixed an important issue.

habibrosyad changed the title ~~panic: offset should be less than or equal to length when inserting emoji~~ Got 'panic: offset should be less than or equal to length' when inserting emoji Dec 1, 2020

hackerwins added the bug 🐞 Something isn't working label Dec 2, 2020

hackerwins added the good first issue 🐤 Good for newcomers label Dec 5, 2020

This was referenced Mar 23, 2021

Fix to operate in consideration of grapheme cluster in Text #163

Closed

Grapheme cluster in Text yorkie-team/yorkie-js-sdk#167

Closed

hackerwins self-assigned this Mar 24, 2021

This was referenced Mar 24, 2021

Fix to operate in consideration of grapheme cluster in Text yorkie-team/yorkie-js-sdk#169

Closed

Calculate the size of Text in UTF-16 code units #165

Merged

hackerwins removed the good first issue 🐤 Good for newcomers label Mar 24, 2021

hackerwins closed this as completed in #165 Mar 27, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Got 'panic: offset should be less than or equal to length' when inserting emoji #110

Got 'panic: offset should be less than or equal to length' when inserting emoji #110

habibrosyad commented Dec 1, 2020

hackerwins commented Dec 2, 2020

habibrosyad commented Dec 2, 2020 •

edited

Loading

habibrosyad commented Dec 2, 2020 •

edited

Loading

hackerwins commented Dec 3, 2020

hackerwins commented Mar 27, 2021

Got 'panic: offset should be less than or equal to length' when inserting emoji #110

Got 'panic: offset should be less than or equal to length' when inserting emoji #110

Comments

habibrosyad commented Dec 1, 2020

hackerwins commented Dec 2, 2020

habibrosyad commented Dec 2, 2020 • edited Loading

habibrosyad commented Dec 2, 2020 • edited Loading

hackerwins commented Dec 3, 2020

hackerwins commented Mar 27, 2021

habibrosyad commented Dec 2, 2020 •

edited

Loading

habibrosyad commented Dec 2, 2020 •

edited

Loading