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

Implement isConnected method #224

Closed
wants to merge 3 commits into from
Closed

Implement isConnected method #224

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aae54f0
Implement isConnected method.
bhartnett Oct 3, 2024
53e9d98
Fix broken tests.
bhartnett Oct 3, 2024
d4d9705
Fix socket server close.
bhartnett Oct 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions json_rpc/client.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,9 @@ method call*(client: RpcClient, name: string,

await client.call(name, params.paramsTx)

method isConnected*(client: RpcClient): bool {.base, gcsafe.} =
doAssert(false, "`RpcClient.isConnected` not implemented")

method close*(client: RpcClient): Future[void] {.base, gcsafe, async.} =
doAssert(false, "`RpcClient.close` not implemented")

Expand Down
4 changes: 4 additions & 0 deletions json_rpc/clients/httpclient.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,12 @@ proc connect*(client: RpcHttpClient, address: string, port: Port, secure: bool)
else:
raise newException(RpcAddressUnresolvableError, res.error)

method isConnected*(client: RpcHttpClient): bool =
(not client.httpSession.isNil()) and client.httpAddress.isOk()

method close*(client: RpcHttpClient) {.async.} =
if not client.httpSession.isNil:
await client.httpSession.closeWait()
client.httpSession = nil
jangko marked this conversation as resolved.
Show resolved Hide resolved

{.pop.}
3 changes: 3 additions & 0 deletions json_rpc/clients/socketclient.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,9 @@ proc connect*(client: RpcSocketClient, address: TransportAddress) {.async.} =
client.address = address
client.loop = processData(client)

method isConnected*(client: RpcSocketClient): bool =
not client.transport.isNil()

method close*(client: RpcSocketClient) {.async.} =
await client.loop.cancelAndWait()
if not client.transport.isNil:
Expand Down
3 changes: 3 additions & 0 deletions json_rpc/clients/websocketclientimpl.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,9 @@ proc connect*(
client.uri = uri
client.loop = processData(client)

method isConnected*(client: RpcWebSocketClient): bool =
not client.transport.isNil()

method close*(client: RpcWebSocketClient) {.async.} =
await client.loop.cancelAndWait()
if not client.transport.isNil:
Expand Down
56 changes: 54 additions & 2 deletions tests/testserverclient.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,51 @@ suite "Socket Server/Client RPC":
except CatchableError as e:
check e.msg == """{"code":-32001,"message":"Unknown payload"}"""

test "Client close and isConnected":
check client.isConnected() == true
# Is socket server close broken?
# waitFor client.close()
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this test when I try to close the socket client it gets stuck waiting and it looks like the socker server loop never exits/finishes for some reason.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a know bug of socket client

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't spend long looking at it. Do you have a suggestion/guidance on how I can fix it? If it isn't trivial, then I'll create an issue for it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is here I think, but I have not actually test it because we do not have production code using socket client or server.

nim-json-rpc/json_rpc/clients/socketclient.nim

Line 85 in be16a65

var value = await client.transport.readLine(defaultMaxRequestLength)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just pushed a fix which solves the problem in the tests. Can you review the change when you get a chance? I'm not sure if it might effect something else.

# check client.isConnected() == false

srv.stop()
waitFor srv.closeWait()

suite "HTTP Server/Client RPC":
var srv = newRpcHttpServer([initTAddress("127.0.0.1", Port(0))])
var client = newRpcHttpClient()

echo "address: ", $srv.localAddress()
srv.setupServer()
srv.start()
waitFor client.connect("http://" & $(srv.localAddress()[0]))

test "Successful RPC call":
let r = waitFor client.call("myProc", %[%"abc", %[1, 2, 3, 4]])
check r.string == "\"Hello abc data: [1, 2, 3, 4]\""

test "Missing params":
expect(CatchableError):
discard waitFor client.call("myProc", %[%"abc"])

test "Error RPC call":
expect(CatchableError): # The error type wont be translated
discard waitFor client.call("myError", %[%"abc", %[1, 2, 3, 4]])

test "Invalid request exception":
try:
discard waitFor client.call("invalidRequest", %[])
check false
except CatchableError as e:
check e.msg == """{"code":-32001,"message":"Unknown payload"}"""

test "Client close and isConnected":
check client.isConnected() == true
waitFor client.close()
check client.isConnected() == false

waitFor srv.stop()
waitFor srv.closeWait()

suite "Websocket Server/Client RPC":
var srv = newRpcWebSocketServer("127.0.0.1", Port(0))
var client = newRpcWebSocketClient()
Expand Down Expand Up @@ -79,6 +121,11 @@ suite "Websocket Server/Client RPC":
except CatchableError as e:
check e.msg == """{"code":-32001,"message":"Unknown payload"}"""

test "Client close and isConnected":
check client.isConnected() == true
waitFor client.close()
check client.isConnected() == false

srv.stop()
waitFor srv.closeWait()

Expand Down Expand Up @@ -111,16 +158,21 @@ suite "Websocket Server/Client RPC with Compression":
except CatchableError as e:
check e.msg == """{"code":-32001,"message":"Unknown payload"}"""

test "Client close and isConnected":
check client.isConnected() == true
waitFor client.close()
check client.isConnected() == false

srv.stop()
waitFor srv.closeWait()

suite "Custom processClient":
test "Should be able to use custom processClient":
var wasCalled: bool = false

proc processClientHook(server: StreamServer, transport: StreamTransport) {.async: (raises: []), gcsafe.} =
wasCalled = true

var srv = newRpcSocketServer(processClientHook)
srv.addStreamServer("localhost", Port(8888))
var client = newRpcSocketClient()
Expand Down
Loading