Skip to content

Commit

Permalink
fix multi-goroutine request error (#54)
Browse files Browse the repository at this point in the history
* fix multi-goroutine bug and add multi-goroutine request test
1. panic: concurrent write to websocket connection
2. request GetSceneList: mismatched ID: expected response with ID c30f63f4-0065-4cb0-5873-19e12a676ee1, but got 97362273-a871-415b-6610-45894239012e

* generate goroutine test

* these tests actually don't have to be generated

put them in a separate file

* add goroutine test

---------

Co-authored-by: Andrey Kaipov <andreykaipov@users.noreply.github.com>
  • Loading branch information
ImmortalD and andreykaipov authored Sep 15, 2023
1 parent ae9a1ff commit ec56dc7
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 70 deletions.
5 changes: 5 additions & 0 deletions api/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package api
import (
"encoding/json"
"fmt"
"sync"
"time"

"github.com/andreykaipov/goobs/api/opcodes"
Expand All @@ -24,6 +25,7 @@ type Client struct {
IncomingResponses chan *opcodes.RequestResponse
Opcodes chan opcodes.Opcode
Log Logger
mutex sync.Mutex
}

// SendRequest abstracts the logic every subclient uses to send a request and
Expand Down Expand Up @@ -56,6 +58,9 @@ func (c *Client) SendRequest(requestBody Params, responseBody interface{}) error

c.Log.Printf("[INFO] Sending %s Request with ID %s", name, id)

c.mutex.Lock()
defer c.mutex.Unlock()

c.Opcodes <- &opcodes.Request{
Type: name,
ID: id,
Expand Down
54 changes: 54 additions & 0 deletions client_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package goobs_test

import (
"net"
"net/http"
"os"
"sync"
"testing"

goobs "github.com/andreykaipov/goobs"
"github.com/gorilla/websocket"
"github.com/stretchr/testify/assert"
)

func Test_client(t *testing.T) {
var err error
_, err = goobs.New(
"localhost:"+os.Getenv("OBS_PORT"),
goobs.WithPassword("wrongpassword"),
goobs.WithRequestHeader(http.Header{"User-Agent": []string{"goobs-e2e/0.0.0"}}),
)
assert.Error(t, err)
assert.IsType(t, &websocket.CloseError{}, err)
assert.Equal(t, err.(*websocket.CloseError).Code, 4009)
_, err = goobs.New(
"localhost:42069",
goobs.WithPassword("wrongpassword"),
goobs.WithRequestHeader(http.Header{"User-Agent": []string{"goobs-e2e/0.0.0"}}),
)
assert.Error(t, err)
assert.IsType(t, &net.OpError{}, err)
}

func Test_multi_goroutine(t *testing.T) {
client, err := goobs.New(
"localhost:"+os.Getenv("OBS_PORT"),
goobs.WithPassword("goodpassword"),
goobs.WithRequestHeader(http.Header{"User-Agent": []string{"goobs-e2e/0.0.0"}}),
)
assert.NoError(t, err)
t.Cleanup(func() {
client.Disconnect()
})
wg := sync.WaitGroup{}
for i := 0; i < 1000; i++ {
wg.Add(1)
go func() {
defer wg.Done()
_, err := client.Scenes.GetSceneList()
assert.NoError(t, err)
}()
}
wg.Wait()
}
34 changes: 0 additions & 34 deletions internal/generate/tests/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,6 @@ func main() {
f := NewFile("goobs_test")
f.HeaderComment("This file has been automatically generated. Don't edit it.")

f.Add(generateClientTest().Line())

// find subclients at the root of goobs, using that to then find the
// structs within each of the subclient categories

Expand Down Expand Up @@ -114,38 +112,6 @@ func main() {
}
}

func generateClientTest() *Statement {
s := Line()

s.Func().Id("Test_client").Params(Id("t").Op("*").Qual("testing", "T")).Block(
Var().Id("err").Error(),
// tries to connect incorrectly
List(Id("_"), Id("err")).Op("=").Qual(goobs, "New").Call(
Lit("localhost:").Op("+").Qual("os", "Getenv").Call(Lit("OBS_PORT")),
Qual(goobs, "WithPassword").Call(Lit("wrongpassword")),
Qual(goobs, "WithRequestHeader").Call(Qual("net/http", "Header").Values(Dict{
Lit("User-Agent"): Index().String().Values(Lit("goobs-e2e/0.0.0")),
})),
),
Qual(assert, "Error").Call(Id("t"), Id("err")),
Qual(assert, "IsType").Call(Id("t"), Op("&").Qual(websocket, "CloseError").Block(), Id("err")),
Qual(assert, "Equal").Call(Id("t"), Id("err").Assert(Op("*").Qual(websocket, "CloseError")).Dot("Code"), Lit(4009)),

// tries to connect to a nonrunning server
List(Id("_"), Id("err")).Op("=").Qual(goobs, "New").Call(
Lit("localhost:42069"),
Qual(goobs, "WithPassword").Call(Lit("wrongpassword")),
Qual(goobs, "WithRequestHeader").Call(Qual("net/http", "Header").Values(Dict{
Lit("User-Agent"): Index().String().Values(Lit("goobs-e2e/0.0.0")),
})),
),
Qual(assert, "Error").Call(Id("t"), Id("err")),
Qual(assert, "IsType").Call(Id("t"), Op("&").Qual("net", "OpError").Block(), Id("err")),
)

return s
}

func generateRequestTest(subclient, category string, structs map[string]StructFieldMap) *Statement {
paramsMapper := func(request, field, fieldType string) *Statement {
var val *Statement
Expand Down
32 changes: 17 additions & 15 deletions test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ setup() {
echo "Main OBS container is already running"
else
docker run --rm --detach --name obs -e vnc=1 -p 5900:5900 -p 4455:1234 ghcr.io/andreykaipov/goobs
sleep 3
fi

# record and stream categories aren't totally idempotent so we need
Expand All @@ -26,7 +27,7 @@ setup() {

gotest() {
category="$1"
go test -v -run="^Test_$category$" -coverprofile=cover.out -coverpkg=./... -covermode=$covermode ./zz_generated.*test.go
go test -v -run="^Test_$category$" -coverprofile=cover.out -coverpkg=./... -covermode=$covermode ./...
awk 'NR>1' cover.out >>coverall.out
}

Expand All @@ -39,20 +40,21 @@ main() {
# note: `scenes` and `transitions` must be ran after `ui`
#
categories='
client
config
filters
general
inputs
mediainputs
outputs
rconfig
sceneitems
sources
ui
scenes
transitions
'
client
multi_goroutine
config
filters
general
inputs
mediainputs
outputs
rconfig
sceneitems
sources
ui
scenes
transitions
'

for category in $categories; do
echo "$category"
Expand Down
21 changes: 0 additions & 21 deletions zz_generated._test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
package goobs_test

import (
"net"
"net/http"
"os"
"testing"
Expand All @@ -23,29 +22,9 @@ import (
transitions "github.com/andreykaipov/goobs/api/requests/transitions"
ui "github.com/andreykaipov/goobs/api/requests/ui"
typedefs "github.com/andreykaipov/goobs/api/typedefs"
websocket "github.com/gorilla/websocket"
assert "github.com/stretchr/testify/assert"
)

func Test_client(t *testing.T) {
var err error
_, err = goobs.New(
"localhost:"+os.Getenv("OBS_PORT"),
goobs.WithPassword("wrongpassword"),
goobs.WithRequestHeader(http.Header{"User-Agent": []string{"goobs-e2e/0.0.0"}}),
)
assert.Error(t, err)
assert.IsType(t, &websocket.CloseError{}, err)
assert.Equal(t, err.(*websocket.CloseError).Code, 4009)
_, err = goobs.New(
"localhost:42069",
goobs.WithPassword("wrongpassword"),
goobs.WithRequestHeader(http.Header{"User-Agent": []string{"goobs-e2e/0.0.0"}}),
)
assert.Error(t, err)
assert.IsType(t, &net.OpError{}, err)
}

func Test_config(t *testing.T) {
client, err := goobs.New(
"localhost:"+os.Getenv("OBS_PORT"),
Expand Down

0 comments on commit ec56dc7

Please sign in to comment.