Add typing notifications to /sync responses - fixes #635 #718

Cnly · 2019-06-18T13:16:03Z

This PR adds a new consumer for typing notifications in syncapi. It also brings changes to syncserver.go and some related files so EDUs can better fit in /sync responses.

Fixes #635.
Fixes #574.

Signed-off-by: Alex Chen minecnly@gmail.com

Pull Request Checklist

I have added any new tests that need to pass to testfile as specified in docs/sytest.md
Pull request includes a sign off

Signed-off-by: Alex Chen <minecnly@gmail.com>

anoadragon453 · 2019-06-25T13:35:43Z

Could you fix the linting? Cyclomatic complexity just means the function has too many branching statements (it is indeed annoying but helps keep functions to a reasonable number of lines).

Also during testing I received a stack trace whenever sending a typing event:

INFO[2019-06-25T13:31:27.257914853Z] Incoming request                              req.id=hsiy5pdFCqqy req.method=PUT req.path="/_matrix/client/r0/rooms/!aaLEoOz66BAshy1K:localhost/typing/@bla:localhost"
ERRO[2019-06-25T13:31:27.258729695Z] Request panicked!
goroutine 130 [running]:
runtime/debug.Stack(0xc000392cc0, 0xc0002192b8, 0xc000392d20)
	/usr/lib/go-1.11/src/runtime/debug/stack.go:24 +0xa7
github.com/matrix-org/util.Protect.func1.1(0xc0005c2700, 0xc05ec0, 0xc000652c40)
	/home/user/go/pkg/mod/github.com/matrix-org/util@v0.0.0-20171127121716-2e2df66af2f5/json.go:87 +0x16f
panic(0xa6a140, 0x1070190)
	/usr/lib/go-1.11/src/runtime/panic.go:513 +0x1b9
github.com/matrix-org/dendrite/typingserver/cache.(*TypingCache).GetTypingUsersIfUpdatedAfter(0xc000251b30, 0xc00001cab9, 0x1b, 0x0, 0x8, 0xc0002196c0, 0xc000219698, 0x8)
	/home/user/code/dendrite/typingserver/cache/cache.go:66 +0x8a
github.com/matrix-org/dendrite/typingserver/cache.(*TypingCache).GetTypingUsers(0xc000251b30, 0xc00001cab9, 0x1b, 0xc000032000, 0x203000, 0x203000)
	/home/user/code/dendrite/typingserver/cache/cache.go:53 +0x48
github.com/matrix-org/dendrite/typingserver/input.(*TypingServerInputAPI).sendEvent(0xc000251b60, 0xc000280a80, 0xe, 0xc00001cab9)
	/home/user/code/dendrite/typingserver/input/input.go:60 +0x5d
github.com/matrix-org/dendrite/typingserver/input.(*TypingServerInputAPI).InputTypingEvent(0xc000251b60, 0xc06b00, 0xc00044b6b0, 0xc000280a80, 0x109bae8, 0xa03020, 0xc00018c9f0)
	/home/user/code/dendrite/typingserver/input/input.go:56 +0xff
github.com/matrix-org/dendrite/clientapi/producers.(*TypingServerProducer).Send(0xc0002824a0, 0xc06b00, 0xc00044b6b0, 0xc00001cadc, 0xe, 0xc00001cab9, 0x1b, 0xc00001ca00, 0x0, 0xc00001cab9, ...)
	/home/user/code/dendrite/clientapi/producers/typingserver.go:49 +0x1bb
github.com/matrix-org/dendrite/clientapi/routing.SendTyping(0xc0005c2800, 0xc00044b770, 0xc00001cab9, 0x1b, 0xc00001cadc, 0xe, 0xc0001fe0f0, 0xc0002824a0, 0xb36e12, 0x6, ...)
	/home/user/code/dendrite/clientapi/routing/sendtyping.go:70 +0x2bb
github.com/matrix-org/dendrite/clientapi/routing.Setup.func17(0xc0005c2800, 0xc00044b770, 0xc0001fe0f0, 0xbff7e0, 0xc00019a960, 0x0)
	/home/user/code/dendrite/clientapi/routing/routing.go:187 +0x128
github.com/matrix-org/dendrite/common.MakeAuthAPI.func1(0xc0005c2800, 0x0, 0x0, 0x0, 0x0)
	/home/user/code/dendrite/common/httpapi.go:27 +0xb6
github.com/matrix-org/util.(*jsonRequestHandlerWrapper).OnIncomingRequest(0xc000196518, 0xc0005c2800, 0xc00012f680, 0x80, 0x80, 0xb05c20)
	/home/user/go/pkg/mod/github.com/matrix-org/util@v0.0.0-20171127121716-2e2df66af2f5/json.go:68 +0x33
github.com/matrix-org/util.MakeJSONAPI.func1(0xc05ec0, 0xc000652c40, 0xc0005c2800)
	/home/user/go/pkg/mod/github.com/matrix-org/util@v0.0.0-20171127121716-2e2df66af2f5/json.go:128 +0x89
github.com/matrix-org/util.Protect.func1(0xc05ec0, 0xc000652c40, 0xc0005c2700)
	/home/user/go/pkg/mod/github.com/matrix-org/util@v0.0.0-20171127121716-2e2df66af2f5/json.go:92 +0x8b
net/http.HandlerFunc.ServeHTTP(0xc000283350, 0xc05ec0, 0xc000652c40, 0xc0005c2700)
	/usr/lib/go-1.11/src/net/http/server.go:1964 +0x44
github.com/matrix-org/dendrite/common.MakeExternalAPI.func1(0xc05ec0, 0xc000652c40, 0xc0005c2600)
	/home/user/code/dendrite/common/httpapi.go:40 +0x190
net/http.HandlerFunc.ServeHTTP(0xc0002e6280, 0xc05ec0, 0xc000652c40, 0xc0005c2600)
	/usr/lib/go-1.11/src/net/http/server.go:1964 +0x44
github.com/gorilla/mux.(*Router).ServeHTTP(0xc00019a780, 0xc05ec0, 0xc000652c40, 0xc0005c2600)
	/home/user/go/pkg/mod/github.com/gorilla/mux@v1.3.0/mux.go:114 +0xe0
github.com/matrix-org/dendrite/common.WrapHandlerInCORS.func1(0xc05ec0, 0xc000652c40, 0xc0005c2400)
	/home/user/code/dendrite/common/httpapi.go:114 +0x169
net/http.HandlerFunc.ServeHTTP(0xc0004a8700, 0xc05ec0, 0xc000652c40, 0xc0005c2400)
	/usr/lib/go-1.11/src/net/http/server.go:1964 +0x44
net/http.(*ServeMux).ServeHTTP(0x107dc40, 0xc05ec0, 0xc000652c40, 0xc0005c2400)
	/usr/lib/go-1.11/src/net/http/server.go:2361 +0x127
net/http.serverHandler.ServeHTTP(0xc0004b60d0, 0xc05ec0, 0xc000652c40, 0xc0005c2400)
	/usr/lib/go-1.11/src/net/http/server.go:2741 +0xab
net/http.(*conn).serve(0xc0000a6140, 0xc06a40, 0xc0001922c0)
	/usr/lib/go-1.11/src/net/http/server.go:1847 +0x646
created by net/http.(*Server).Serve
	/usr/lib/go-1.11/src/net/http/server.go:2851 +0x2f5  context=missing panic="runtime error: invalid memory address or nil pointer dereference"
INFO[2019-06-25T13:31:27.258879396Z] Responding (35 bytes)                         code=500 context=missing

Signed-off-by: Alex Chen <minecnly@gmail.com>

Cnly · 2019-06-25T14:58:33Z

Linting, complexity and nil pointer dereference should all be fixed now.

Signed-off-by: Alex Chen <minecnly@gmail.com>

…r and notifier Signed-off-by: Alex Chen <minecnly@gmail.com>

…ypingEvent Signed-off-by: Alex Chen <minecnly@gmail.com>

Previously, currPos, obtained using notifier.CurrentPosition(), is passed to GetNotifyChannel. Since this is already the latest position on the server, the client won't get updates until (1) a even newer event arrives at the server, or (2) the /sync request times out. (2) above is possible because when it times out, notifier calculates currentSyncForUser based on syncReq, which contains the correct since pos token that the client sent the server. Signed-off-by: Alex Chen <minecnly@gmail.com>

Signed-off-by: Alex Chen <minecnly@gmail.com>

…elta") Signed-off-by: Alex Chen <minecnly@gmail.com>

Signed-off-by: Alex Chen <minecnly@gmail.com>

Cnly · 2019-06-27T17:34:55Z

Just refined some docs. The PR is now ready for review! :)

Signed-off-by: Alex Chen <minecnly@gmail.com>

syncapi/sync/notifier.go

syncapi/types/types.go

syncapi/consumers/typingserver.go

syncapi/sync/notifier.go

syncapi/sync/request.go

syncapi/types/types.go

typingserver/input/input.go

anoadragon453 · 2019-07-11T13:27:15Z

Also, please merge master into this branch.

(The fact testfile is so easy to conflict on makes me a little worried).

Signed-off-by: Alex Chen <minecnly@gmail.com>

anoadragon453

lgtm! other than just some small grammatical things

syncapi/storage/syncserver.go

syncapi/sync/notifier.go

typingserver/cache/cache.go

Co-Authored-By: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>

In 29841be (matrix-org#718), EDUs are added to /sync responses for rooms listed in joinedRoomIDs returned by addPDUDeltaToResponse. However this list may contain rooms other than those currently joined. Some variable renamings are done to make golangci-lint pass. Signed-off-by: Alex Chen <minecnly@gmail.com>

In 29841be (#718), EDUs are added to /sync responses for rooms listed in joinedRoomIDs returned by addPDUDeltaToResponse. However this list may contain rooms other than those currently joined. Some variable renamings are done to make golangci-lint pass. Signed-off-by: Alex Chen minecnly@gmail.com

…org#752) In 29841be (matrix-org#718), EDUs are added to /sync responses for rooms listed in joinedRoomIDs returned by addPDUDeltaToResponse. However this list may contain rooms other than those currently joined. Some variable renamings are done to make golangci-lint pass. Signed-off-by: Alex Chen minecnly@gmail.com

Add typing notifications to /sync responses - fixes matrix-org#635

b896fdc

Signed-off-by: Alex Chen <minecnly@gmail.com>

Cnly force-pushed the syncapi-typing-notifications-635 branch from f056bec to b896fdc Compare June 25, 2019 08:01

Cnly added 3 commits June 25, 2019 22:22

Fix linting

55a4e60

Signed-off-by: Alex Chen <minecnly@gmail.com>

Fix nil pointer and goroutine safety in typing cache

ba35a0a

Signed-off-by: Alex Chen <minecnly@gmail.com>

Add warning for OnNewEvent when no user to wake up

e742b7d

Signed-off-by: Alex Chen <minecnly@gmail.com>

Cnly added 14 commits June 26, 2019 17:45

Fix syncapi/sync/notifier_test.go

e687027

Signed-off-by: Alex Chen <minecnly@gmail.com>

Fix more linting issues and docs

18c59e6

Signed-off-by: Alex Chen <minecnly@gmail.com>

Add a simple test for EDU-only updates in notifier_test.go

badd360

Signed-off-by: Alex Chen <minecnly@gmail.com>

Remove room ID from m.typing client events

ad4f69a

Signed-off-by: Alex Chen <minecnly@gmail.com>

Typing event consumer now supplies real, not bogus position to notifier

d3ee72c

Signed-off-by: Alex Chen <minecnly@gmail.com>

Better logging in typing event consumer

55219aa

Signed-off-by: Alex Chen <minecnly@gmail.com>

Fix sync position with partial info used as complete one in syncserve…

2a9dab2

…r and notifier Signed-off-by: Alex Chen <minecnly@gmail.com>

Fix necessary field not set properly in TypingEvent; clean up OutputT…

e7cf449

…ypingEvent Signed-off-by: Alex Chen <minecnly@gmail.com>

Make Notifier.CurrentPosition() respect stream lock

aeceacd

Signed-off-by: Alex Chen <minecnly@gmail.com>

Fix docs for SyncPosition.WithUpdates

6fd1c21

Signed-off-by: Alex Chen <minecnly@gmail.com>

Fix zero value of currPos may be used in OnIncomingSyncRequest

274db2f

Signed-off-by: Alex Chen <minecnly@gmail.com>

Userstreams should store and use complete pos, not pos update ("pos d…

d79a6a2

…elta") Signed-off-by: Alex Chen <minecnly@gmail.com>

Remove users from typing list when typing status times out

19f1a6f

Signed-off-by: Alex Chen <minecnly@gmail.com>

anoadragon453 requested a review from a team June 27, 2019 17:20

anoadragon453 self-assigned this Jun 27, 2019

Refine docs

530ed11

Signed-off-by: Alex Chen <minecnly@gmail.com>

Cnly added 2 commits July 1, 2019 19:38

Add newly passing tests to testfile

1be2089

Signed-off-by: Alex Chen <minecnly@gmail.com>

Merge branch 'master' into syncapi-typing-notifications-635

8ed82c0

Signed-off-by: Alex Chen <minecnly@gmail.com>

anoadragon453 reviewed Jul 11, 2019

View reviewed changes

Docs and code cleanup

4e84c2f

Signed-off-by: Alex Chen <minecnly@gmail.com>

Merge 'upstream/master' into syncapi-typing-notifications-635

4a323f1

Signed-off-by: Alex Chen <minecnly@gmail.com>

Cnly requested a review from anoadragon453 July 12, 2019 12:52

anoadragon453 approved these changes Jul 12, 2019

View reviewed changes

Cnly and others added 2 commits July 12, 2019 22:26

Apply suggestions from code review

24b3e1a

Co-Authored-By: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>

Apply suggestions from code review

801c664

Co-Authored-By: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>

Cnly merged commit 29841be into matrix-org:master Jul 12, 2019

Cnly mentioned this pull request Jul 19, 2019

Fix /sync may contain duplicate EDUs and EDUs for left rooms #752

Merged

2 tasks

Cnly added C-Sync-API C-EDU-Server labels Aug 25, 2019

matrixbot mentioned this pull request Oct 31, 2024

Add typing notifications to /sync responses - fixes #635 element-hq/dendrite#718

Closed

Uh oh!

Add typing notifications to /sync responses - fixes #635 #718

Add typing notifications to /sync responses - fixes #635 #718

Uh oh!

Conversation

Cnly commented Jun 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Checklist

Uh oh!

anoadragon453 commented Jun 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Cnly commented Jun 25, 2019

Uh oh!

Cnly commented Jun 27, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anoadragon453 commented Jul 11, 2019

Uh oh!

anoadragon453 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Cnly commented Jun 18, 2019 •

edited

Loading

anoadragon453 commented Jun 25, 2019 •

edited

Loading