Golang tcp socket getting stuck into io.EOF error

newbiegolang · August 5, 2022, 6:19pm

What I am trying to achieve is to build a golang socket server. This server will receive data from connected devices. The devices will send data in hex format example

000000000000006008010000018063BE5660003BD7AAC40337AAFCFFED00B5090015001609EF01F00150011503C80045010101B300713E0A42392D180015430E6F44008C0900AD0D029E11003912FFDA13FFFE0F03E803C700000697100008D9E70C0000895600010000E5F2

Here is my snippet of java codes which it works perfectly fine.

while (true) {  
    if((firstByte=r.readByte()) != -1){
        if (firstByte == 0x00) {
          // rest of the logic 
        }
    }

So in java what I does is that once device is connected then I ready byte by byte then I got a certain logic to it to process a group of bytes. I am trying to achieve the same as below is my golang codes.

package main
import (
        "fmt"
        "net"
        "os"
        "bufio"
        "io"
)
const (
        SERVER_HOST = "localhost"
        SERVER_PORT = "9999"
        SERVER_TYPE = "tcp"
)
func main() {
        fmt.Println("Server Running...")
        server, err := net.Listen("tcp", ":9999")
        if err != nil {
                fmt.Println("Error listening:", err.Error())
                os.Exit(1)
        }
        defer server.Close()
        fmt.Println("Listening on " + SERVER_HOST + ":" + SERVER_PORT)
        fmt.Println("Waiting for client...")
        for {
                connection, err := server.Accept()
                if err != nil {
                        fmt.Println("Error accepting: ", err.Error())
                        os.Exit(1)
                }
                fmt.Println("client connected")
                go processClient(connection)
        }
}
func processClient(connection net.Conn) {
    r := bufio.NewReader(connection)
    for {
        //a := -1

        line, err := r.ReadBytes(byte(0xff))
       switch err {
        case nil:
            break
        case io.EOF:fmt.Println("IO  EOF", err)
        default:
            fmt.Println("ERROR", err)
        }
		//str:=string(line)
        // do something with the data, in this case echo it back
		fmt.Printf(" TRY PINT NOW %s\n", line)
		//fmt.Println("Output binary is what we have read so far %v \n", line)
		//fmt.Println("Output hex is what we have read so far %x \n", line)
        //connection.Write(line)
    }
    connection.Close()

}

Can any one help me what is my mistake in my golang version I am trying to replicate the same basic logic first then I will go further to complete it.

skillian · August 6, 2022, 11:52am

I copied your code out and ran it and it looks like the problem is that after the request is processed and you get an EOF, your program keeps printing

IO  EOF EOF
 TRY PINT NOW

forever. This is because you’re not breaking out of your for { ... } loop, so r.ReadBytes keeps getting called and is at EOF every time. I restructured the error handling a little:

		line, err := r.ReadBytes(byte(0xff))
		if errors.Is(err, io.EOF) {
			fmt.Println("IO  EOF", err)
			break
		}
		if err != nil {
			fmt.Println("ERROR", err)
			break
		}

And now the code breaks when there’s an error.

newbiegolang · August 11, 2022, 4:43pm

Hi Skillian,
Thank you I shall try you solution. Anyway what is the right place to close this connection is it defer.close at the very top line or should I leave it as the bottom?

skillian · August 12, 2022, 12:31pm

Good question: I think my suggestion would be to put it in a defer so that if any unexpected panic occurs in your connection handler (maybe if/when you add additional function calls), you can be sure that the connection will be closed. The only time I see cleanup code not in a defer is in performance-critical code (e.g. some code that locks and unlocks mutexes). But in the case of I/O, like closing a network connection, I think it’s best to defer it.

newbiegolang · August 12, 2022, 2:55pm

Hi Skillian,
Sorry I am a bit new. I notice like db, socket connection etc. a lot of example are using defer.close(). I still dont get it if we do normal close versus defer what is the benefit ? Also I notice defer it put at the very top after you open the connection right? So in this case is this correct place to close ?

func processClient(connection net.Conn) {
defer.close()
    r := bufio.NewReader(connection)
    for {
        //a := -1

        line, err := r.ReadBytes(byte(0xff))
       switch err {
        case nil:
            break
        case io.EOF:fmt.Println("IO  EOF", err)
        default:
            fmt.Println("ERROR", err)
        }
		//str:=string(line)
        // do something with the data, in this case echo it back
		fmt.Printf(" TRY PINT NOW %s\n", line)
		//fmt.Println("Output binary is what we have read so far %v \n", line)
		//fmt.Println("Output hex is what we have read so far %x \n", line)
        //connection.Write(line)
    }
    

}

skillian · August 12, 2022, 3:06pm

Check out Go by Example: Defer. If you’re familiar with other languages like Python, C#, or Java, defer takes the place of try ... finally in those languages. In a language like Java, I think you would write something like this (disclaimer: I don’t actually know Java):

class ConnectionProcessor {
    public void processClient(connection Connection) {
        try {
            // function body
        } finally {
            connection.close()
        }
    }
}

In Go, you would instead do:

func processClient(connection net.Conn) {
    defer connection.Close()
    // function body
}

My understanding is that the motivation for this is to make sure that cleanup is always specified right around where a resource is acquired. For example, files also have to be explicitly closed, so you can do something like this:

func doSomethingWithFilename(filename string) error {
    f, err := os.Open(filename)
    if err != nil {
        return fmt.Errorf("failed to open %q: %w", filename, err)
    }
    defer f.Close()
    // do something with the File, f.
    // if this block of code is very long, you don't have to worry about
    // forgetting to close f because it's done right after f is opened.
}

newbiegolang · August 12, 2022, 3:09pm

Hi Skillian,
Thank you I just finish reading about defer in depth yet it powerful to do the final clean up. So my placing of the defer it correct right in my case. Yes I am from java background so it easier to understand and relate like you mention the try and catch? I am going to expand further on your solution. Say in my case I need to also be using database connection. I guess it best to build them in the main. Then do defer close in the processClient right?

skillian · August 12, 2022, 3:38pm

I would recommend that, wherever it is possible, you put the defer in the same function as wherever the resource is created. Like in my doSomethingWithFilename example: The file is opened and immediately after, a call to (*os.File).Close is deferred.

When you separate where a resource is created from where it is cleaned up, it makes things complicated. For example, I think this code is not good:

func OpenAndProcessFile(filename string) error {
    f, err := os.Open(filename)
    if err != nil {
        return fmt.Errorf("failed to open %q: %w", filename, err)
    }
    return ProcessFile(f)
}

func ProcessFile(file *os.File) error {
    defer file.Close()
    // do something with the file
}

What happens if the process changes slightly in the future? Maybe OpenAndProcessFile needs to check something in the file and call ProcessFile2 instead of ProcessFile:

if something {
    return ProcessFile2(f)
}
return ProcessFile(f)

Now ProcessFile2 also needs to close the file.

And then what if there is some other condition that requires both ProcessFile and ProcessFile2 to be called?

if something {
    return ProcessFile2(f)
}
if something2 {
    if err := ProcessFile2(f); err != nil {
        return err
    }
}
return ProcessFile(f)

But this won’t work because ProcessFile2 closes the file, so when ProcessFile gets called, it will produce an error because the file is closed. You might have to make ProcessFile more complicated because now it needs to re-open the file if it’s been passed a closed file.

All of this complexity is unnecessary. A better solution would be:

func OpenAndProcessFile(filename string) error {
    f, err := os.Open(filename)
    if err != nil {
        return fmt.Errorf("failed to open %q: %w", filename, err)
    }
    defer f.Close()
    if something {
        return ProcessFile2(f)
    }
    if something2 {
        if err := ProcessFile2(f); err != nil {
            return err
        }
    }
    return ProcessFile(f)
}

func ProcessFile(f *os.File) error {
    // do something with f, but don't close it
}

func ProcessFile2(f *os.File) error {
    // do something with f, but don't close it
}

Now ProcessFile and ProcessFile2 don’t need to know about closing and/or opening files. It’s all handled by the caller. Opening and closing the file is handled in only one place.

So,

To answer your question:

I would suggest:

Open your database connection in main and (if necessary) defer closing it in main.
Pass the database connection to processClient and don’t worry about closing the database connection in processClient

For example, in your example’s main function, I would consider refactoring to:

func main() {
        db := mustOpenDatabaseConnection()
        defer db.Close()
        // ...
        for {
                connection, err := server.Accept()
                if err != nil {
                        fmt.Println("Error accepting: ", err.Error())
                        os.Exit(1)
                }
                fmt.Println("client connected")
                go func() {
                        defer connection.Close()
                        processClient(connection, db)
                }()
        }

// ...

func processClient(connection net.Conn, db *sql.DB) {
    // don't defer closing the connection or database here;
    // it's handled by the caller
}

A benefit here is that now processClient doesn’t need to participate in the connection or database’s lifetimes. They are “nested” in the call stack instead of being passed through it.

newbiegolang · August 12, 2022, 3:50pm

Hi Skillian,
Great your examples are very clear and giving more details into defer. But for the db. So what what I read is like this they said that the db creation will create a pool of connection and then in the function is good to close the pool to return but there are some contradicting saying no need to close.

Back you first solution you gave me I am testing some additional stuff.

 r := bufio.NewReader(connection)

	for {
		firstByte, err1 := r.ReadBytes(byte(0xff))
		
		if firstByte == byte(0x00) {
				// rest of the logic
                                fmt.Println("FIRST IS OOOOOO") 
                }

                if errors.Is(err1, io.EOF) {
			fmt.Println("IO  EOF", err1)
			break
		}
		if err1 != nil {
			fmt.Println("ERROR", err1)
			break
		}
      }

I have few issue here first I go this error invalid operation: firstByte == byte(0) (mismatched types []byte and byte) I know cause I am reading bytes and comparing it to just a single byte. Based on my current java solution I read byte by byte what is your suggestion for golang ?

Second thing is undefined: errors is your errors a function ? Another thing I learn this codes need to be at the bottom is it

if errors.Is(err1, io.EOF) {
			fmt.Println("IO  EOF", err1)
			break
		}
		if err1 != nil {
			fmt.Println("ERROR", err1)
			break
		}

skillian · August 12, 2022, 4:19pm

Yes, a *sql.DB is a connection pool and if connections from that pool will be used throughout the lifetime of your server process, then there’s no need to open and close them explicitly. The same *sql.DB object can be used concurrently.

You can use (*bufio.Reader).ReadByte instead of (*bufio.Reader).ReadBytes. ReadByte will return a single byte and an error. Based on your example, it looks to me like Java returns -1 when a byte cannot be read, but in Go, you get an error.

errors is the name of a package in the standard library that defines the errors.Is function that I’m using to check if an error indicates an end-of-file (“EOF”) condition. Most code returns io.EOF directly in an end-of-file condition, but I usually use if errors.Is(err, io.EOF) { ... } instead of if err == io.EOF { ... } just in case the error is wrapped somewhere, such as:

func myReader(r io.Reader) error {
    n, err := r.Read(buffer)
    if err == io.EOF {
        // myReader doesn't return io.EOF directly. Instead, it wraps
        // io.EOF into a new error with more information:
        return fmt.Errorf("unexpected EOF while reading %v: %w", r, err)
    }
}

func readSomething(something io.Reader) error {
    if err := myReader(something); err != nil {
        // This function, readSomething, is OK if myReader returns io.EOF.
        // The problem is `if err == io.EOF` below will be `false`
        // because myReader doesn't return io.EOF directly.  Instead, it
        // returns a new error that "wraps" io.EOF (see fmt.Errorf's
        // documentation when it is used with %w) with more information.
        if err == io.EOF {
            // This code will never execute:
            //
            // instead of using `if err == io.EOF`, readSomething should
            // instead use `if errors.Is(err, io.EOF)` which will unwrap
            // err and see that the inner error is io.EOF.
            return nil
        }
        if errors.Is(err, io.EOF) {
            // This code will execute when myReader returns
            // fmt.Errorf("unexpected eof ...")
            return nil
        }
        return err
    }
    // do something else...
}

newbiegolang · August 12, 2022, 5:43pm

Hi Skillian,
Ok I have implemented the error package ready. I dont quite get your last code example of that readSomething and myReader. Anyway I have modified my codes and runs without error as below now. At one glance do you think its ok with Golang on this codes. What I dont understand at time like example

imeiLength, errDeviceImei := r.Read(deviceImeiByteArray)
                                        if errDeviceImei != nil {
                                                if errDeviceImei != io.EOF {
                                                fmt.Println("read error:", errDeviceImei)
                                                }
                                                break
                                        }

In this case I dont even declare imeiLength yet it works ? But at time it tell me undefined error ?

func processClient(connection net.Conn) {
        defer connection.Close()
        r := bufio.NewReader(connection)
        var deviceImei = "" 
	for {
		firstByte, err1 := r.ReadByte()
		if firstByte != 0xff {
				// rest of the logic
                                fmt.Println("FIRST IS OO") 
                                
                                secondByte, err2 := r.ReadByte()

                                if secondByte == 0 {
                                fmt.Println("second IS OOO0") 

                                thirdByte, err3 := r.ReadByte()

                                        if thirdByte == 0 {
                                                fmt.Println("third IS OO0000") 

                                                fourstByte, err4 := r.ReadByte()
                                                if fourstByte == 0 {
                                                        fmt.Println("fourst IS OOOOOO") 
                                                        fmt.Println("Device imei reading fourth") 
                                                        fmt.Println(deviceImei) 

                                                        dataFieldLengthArray := make([]byte, 4) 

                                                        dataFieldLength, errDataFieldLengthArray := r.Read(dataFieldLengthArray)
                                                        if errDataFieldLengthArray != nil {
                                                                fmt.Println("ERROR errDataFieldLengthArray", errDataFieldLengthArray)
                                                                if errDataFieldLengthArray != io.EOF {
                                                                fmt.Println("read error errDataFieldLengthArray:", errDataFieldLengthArray)
                                                                }
                                                                break
                                                        }
                                                        fmt.Println("int  dataFieldLength")
                                                        fmt.Println(dataFieldLength)

                                                        stringDataFieldLengthArray := string(dataFieldLengthArray)
                                                        intDataFieldLength, errStrconvAtoi := strconv.Atoi(stringDataFieldLengthArray)
                                                        fmt.Println("String dataFieldLengthArray")
                                                        fmt.Println(stringDataFieldLengthArray)

                                                        fmt.Println("int intDataFieldLength")
                                                        fmt.Println(intDataFieldLength)

                                                        if errStrconvAtoi != nil {
                                                                fmt.Println("ERROR errStrconvAtoi", errStrconvAtoi)
                                                                if errStrconvAtoi != io.EOF {
                                                                fmt.Println("read error errStrconvAtoi:", errStrconvAtoi)
                                                                }
                                                                break
                                                        }

                                                }
                                                if err4 != nil {
                                                        break
                                                }
                                        }
                                        if err3 != nil {
                                                break
                                        }
                                } else if secondByte == 15 {
                                        fmt.Println("second IS to read imei now") 
                                        deviceImeiByteArray := make([]byte, 15) 

                                        imeiLength, errDeviceImei := r.Read(deviceImeiByteArray)
                                        if errDeviceImei != nil {
                                                if errDeviceImei != io.EOF {
                                                fmt.Println("read error:", errDeviceImei)
                                                }
                                                break
                                        }

                                        deviceImei = string(deviceImeiByteArray[:len(deviceImeiByteArray)])
                                        fmt.Println(deviceImei)
                                        fmt.Println(imeiLength)
                                        fmt.Println("total size:", len(deviceImeiByteArray))

                                }

                                if err2 != nil {
                                        break
                                }
		}

                if errors.Is(err1, io.EOF) {
			fmt.Println("IO  EOF", err1)
			break
		}
		if err1 != nil {
			fmt.Println("ERROR", err1)
			break
		}
		
	}
   }

skillian · August 18, 2022, 11:57pm

It is because you use := instead of just =. x := doSomething() is the same as defining a new x variable with the exact type that doSomething returns, and then settings its value to the result of doSomething(). So if doSomething() returns an int, it is the same thing as var x int = doSomething() but if doSomething() returns an error, then it’s the same thing as var x error = doSomething(). The type of x is deduced from the type of doSomething.

If you use x = doSomething() instead of :=, then you will get an error if x has not already been defined.

I’m not familiar with the protocol that you’re reading, so I cannot be sure that what you’re doing is correct, but I can make some additional suggestions and point out some recommended changes!

We often try to avoid nested ifs, so instead of code like:

if (thing1 == whatWeWant1) {
    if (thing2 == whatWeWant2) {
        if (thing3 == whatWeWant3) {
            // conditions are good:  Run code here.
        } else {
            // handle error 3
        }
    } else {
        // handle error 2
    }
} else {
    // handle error 1
}

We will instead write:

if thing1 != whatWeWant1 {
    // handle error 1
}
if thing2 != whatWeWant2 {
    // handle error 2
}
if thing3 != whatWeWant3 {
    // handle error 3
}

// conditions are good:  Run code here.

So that the “good” condition is not indented and you can read from top to bottom instead of diagonally inward and then back outward again, when looking at the error conditions.

In your code, you have:

dataFieldLengthArray := make([]byte, 4) 
dataFieldLength, errDataFieldLengthArray := r.Read(dataFieldLengthArray)
// ...
stringDataFieldLengthArray := string(dataFieldLengthArray)
intDataFieldLength, errStrconvAtoi := strconv.Atoi(stringDataFieldLengthArray)

Which will read 4 bytes of ASCII text and parse it as a decimal number (e.g. '0', '1', '2', '3' (0x30, 0x31, 0x32, 0x33) would be parsed as the int: 123). Based on how low-level this protocol seems to be, I think what you instead meant was to read 4 bytes out of the stream and interpret those bytes (perhaps in network byte order?) as a 32-bit integer containing a length. If that’s what you wanted, then after reading the bytes into dataFieldLengthArray, you should interpret them as an integer by using encoding/binary.BigEndian.Uint32 which will read out a uint32 in big-endian (“network order”):

dataFieldLengthArray := make([]byte, 4) 
dataFieldLength, errDataFieldLengthArray := r.Read(dataFieldLengthArray)
u32 := binary.BigEndian.Uint32(dataFieldLengthArray)
intDataFieldLength := int(u32)

newbiegolang · August 25, 2022, 5:07pm

Hi Skillian,

I do get your explanation of the x := doSomething() so which is the correct method to go with golang to ensure I don’t get into any issue in future ?

Maybe to give a better idea this the example type of data which I will read sent from the device.

000000000000006008010000018063BE5660003BD7AAC40337AAFCFFED00B5090015001609EF01F00150011503C80045010101B300713E0A42392D180015430E6F44008C0900AD0D029E11003912FFDA13FFFE0F03E803C700000697100008D9E70C0000895600010000E5F2

This example is to fulfil the first criteria where I have now nested those if statement . The issue is I need to read first 00 then next is it 00 then next is 00 next 00. So if the first 4 bytes are all 00 then I start processing this type of data. Else there will be other type of data where first byte is 00 but next one is 0F that where I have this line else if secondByte == 15. Maybe you can suggest an efficient method ?

skillian · August 27, 2022, 2:20pm

Are these two scenarios:

0x00, 0x00, 0x00, 0x00
0x00, 0x0f

The only situations that you’re looking for, or are you going to look for additional “tags?”

newbiegolang · August 27, 2022, 5:48pm

Hi Skillian,
Yes these are 2 different scenario. Yes these are the situations and also few more others too. But to make it simple I want to first focus these 2 types only.

system · November 25, 2022, 5:48pm

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.