search

surrealdb / surrealdb.go

SurrealDB SDK for Golang
https://surrealdb.com
Apache License 2.0
231 stars 62 forks source link

Stabilize WS tests #91

Closed mumoshu closed 1 year ago

mumoshu commented 1 year ago

The tests were failing time to time for me saying this:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x9f9f27]

goroutine 23 [running]:
github.com/surrealdb/surrealdb.go/pkg/gorilla.(*WebSocket).initialize.func1()
    /home/mumoshu/p/surrealdb.go/pkg/gorilla/gorilla.go:205 +0xe7
created by github.com/surrealdb/surrealdb.go/pkg/gorilla.(*WebSocket).initialize
    /home/mumoshu/p/surrealdb.go/pkg/gorilla/gorilla.go:196 +0x69

The paniic points to https://github.com/surrealdb/surrealdb.go/blob/main/pkg/gorilla/gorilla.go#L205, and I see ws.logger being nil when the panic happens.

We should make the logger non-nil, or make the ws.read goroutine wait until the logger is initialized. Otherwise, every time ws.read gets an error from the connection (e.g. the connection is closed?), AND the logger is nil (which is always the case for our no-opt gorilla ws impl in the tests), the panic happens.

That's said, let's always initialize the logger for now.

We may make the logger getting lazily initialized on demand, but I think that's not a priority for now :)

ElecTwix commented 1 year ago

I have identified that there is a bug in the code causing std.out to be used instead of https://github.com/surrealdb/surrealdb.go/blob/main/pkg/logger/logger.go#L51C2-L51C2 when the path is empty. Rest assured, I will create a PR to fix this issue promptly. Thank you for bringing it to my attention.