Network: Better error handling

[Sicro]

SendNetworkString() should have a parameter to set the time-out.
Without this parameter, the server can block the client for a long time, when the server doesn't handle the data.

Better: More error handling

[RichAlgeni]

I don't believe your assertion is correct. If it were, the entire internet would ground to a halt waiting on packet sends to complete.

Post the code you are having an issue with, and we'll take a look at it.

[Sicro]

Other programs are be stopping sending more data, after a while, when the server isn't fast enough.
I think, SendNetworkString does it too, but to late - or it's a endless loop.

Post the code you are having an issue with, and we'll take a look at it.

Example code to have this problem:

Server

Code: Select all

If Not InitNetwork() Or Not CreateNetworkServer(0, 6500)
  Debug "Network error"
  End
EndIf

Repeat
  Event = NetworkServerEvent()
  Select Event
    Case #PB_NetworkEvent_Connect
      Debug "Client has connected."
    Case #PB_NetworkEvent_Data
      ; Client is sending data. The server will block the work of the client by he doesn't handle the incoming data.
    Default
      Delay(10)
  EndSelect
ForEver

Client

Code: Select all

If Not InitNetwork()
  Debug "Network error"
  End
EndIf

Connection = OpenNetworkConnection("localhost", 6500)
If Not Connection
  Debug "Network error"
  End
EndIf

Debug "Begins sending data."
; Now we send a lot of strings, so that the internal buffer is full and he can no longer accept data.
For i = 1 To 200000
  SendNetworkString(Connection, "This is a test string.")
Next
Debug "This will be never seen, because SendNetworkString blocks until the transfer is completed."

[heartbone]

SendNetworkString() should have a parameter to set the time-out.
Without this parameter, the server can block the client for a long time, when the server doesn't handle the data.

Other programs are be stopping sending more data, after a while, when the server isn't fast enough.
I think, SendNetworkString does it too, but to late - or it's a endless loop.

Post the code you are having an issue with, and we'll take a look at it.

Example code to have this problem:
{snip}

Code: Select all

Debug "Begins sending data."
; Now we send a lot of strings, so that the internal buffer is full and he can no longer accept data.
For i = 1 To 200000
  SendNetworkString(Connection, "This is a test string.")
Next
Debug "This will be never seen, because SendNetworkString blocks until the transfer is completed."

Although I comprehend your request for the enhancement,
calling the procedure SendNetworkString() 200,000 times consecutively without any intervening code to produce the conditions necessary to show the problem seems a bit extreme.
That code does not seem represent in any sense of the word, what could be considered a normal problem.
If you could produce the problem after less than 100 such calls, then it might enter the realm of a bug in a somewhat practical scenario.

BTW Sicro, the SendNetworkString() function is buggy the last time I looked at it, it should be used very carefully.
As long as your strings are less than 256 characters then you can directly substitute this code and then see what happens.

Code: Select all

Procedure SNETSTRING(ID,STRING$)
; REPLACES SendNetworkString(ID,STRING$,#PB_UTF8)
   *SBUFFER= AllocateMemory(256)
   PokeS(*SBUFFER,STRING$,Len(STRING$)+1,#PB_UTF8)
   SendNetworkData(ID,*SBUFFER,Len(STRING$)+1)
   FreeMemory(*SBUFFER)
EndProcedure

[HeX0R]

The server will send a wsaewouldblock to the client, if he gets flooded with data and can't handle it fast enough.
Then it's on the client to wait until he gets a clear message from the server.

Unfortunately the error handling of the network library is almost not existant, without API you have no chance to distinguish between all of the possible reasons SendNetworkData failed.

We would need better error handling instead of a timeout parameter.

I'm quite sure I requested this years ago, but can't find it anymore.

[Sicro]

calling the procedure SendNetworkString() 200,000 times consecutively without any intervening code to produce the conditions necessary to show the problem seems a bit extreme.
That code does not seem represent in any sense of the word, what could be considered a normal problem.
If you could produce the problem after less than 100 such calls, then it might enter the realm of a bug in a somewhat practical scenario.

Code: Select all

If Not InitNetwork()
  Debug "Network error"
  End
EndIf

Connection = OpenNetworkConnection("localhost", 6500)
If Not Connection
  Debug "Network error"
  End
EndIf

Debug "Begins sending data."

For i = 1 To 200000
  String.s + "This is a test string."
Next
SendNetworkString(Connection, String)

Debug "This will be never seen, because SendNetworkString blocks until the transfer is completed."

Another way:
1. Send data with SendNetworkData() to get the internal buffer nearly full.
2. Send a string with SendNetworkString() to overfill the buffer
3. Client stops working

The problem is not only with SendNetworkString(), but also at SendNetworkData().
When the internal buffer is full, also blocks SendNetworkData().

Code: Select all

If Not InitNetwork()
  Debug "Network error"
  End
EndIf

Connection = OpenNetworkConnection("localhost", 6500)
If Not Connection
  Debug "Network error"
  End
EndIf

Debug "Begins sending data."

*FileData = AllocateMemory(1024*1024*10) ; 10 MiB fails, 1 MiB ok
DataSize = MemorySize(*FileData)
Offset = 0
Repeat
  SentDataSize = SendNetworkData(Connection, *FileData+Offset, DataSize-Offset)
  If SentDataSize > 0
    Offset + SentDataSize
  EndIf
Until Offset = DataSize

Debug "This will be never seen, because SendNetworkData blocks until the transfer is completed." 

@HeX0R: Yes, more error handling will be better. I have changed the title of the topic.

[RichAlgeni]

One other tip I would suggest it to trigger your reads from the server side with NetworkServerEvent() on #PB_NetworkEvent_Connect, and not #PB_NetworkEvent_Data IF you will be using threads to service the sockets.

I have found that multiple instances of #PB_NetworkEvent_Data per connection can be triggered if your loop is tight enough, but only one instance per connection of #PB_NetworkEvent_Connect will be triggered.'

Also, never close a server socket, allow this system to do so. But always close client sockets!