Golang server app not writing to file

Getting Help Code Review
1

I’m making a simple web server that writes all tcp packets recieved to output.txt, so I wrote this:

func handleRequest(conn net.Conn) {
    // Make a buffer to hold incoming data.
    buf := make([]byte, 1024)
    // Read the incoming connection into the buffer.
    size, err := conn.Read(buf)
    if err != nil {
        fmt.Println(“Error reading:”, err.Error())
    }
    s := string(buf[:size])
    fmt.Println(s)
    perm := os.FileMode(0644)
    err = ioutil.WriteFile(“output.txt”, buf[:size], perm)
    check(err)
    // Close the connection when you’re done with it. 
    conn.Close()
}

It makes output.txt, but writes nothing in it. All the “server part” works fine(it outputs s correctly)

2

Does check create any output? I can’t find the definition of it in your code…

Also this board uses markdown style code formatting, so you have to either indent your code by 4 spaces or wrap it into triple-backticks with optional language annotation:

```go
// your code goes here
```

BBcode doesn’t work here.

Besides of the question with the error checking and the markdown, I see some problems with your code:

  1. It will probably fail when you receive more than 1024 bytes at once, or at least it might not consume them properly.
  2. You are closing a connection that might still get used on the call side.
3

Those limitations are made on purpose because of the device that sends TCP connections

4

But what about your check function? As what I can see from your code, it looks like you will continue with reporting success even when there was an error during write, that’s why I want to see your check function.

edit

Even though you say that you data will not exceed 1kByte, it might get handed over to you in a chunked mannor. You should therefore define specify a clear protocol that adheres to those circumstances.

Also I am still the opinion, that closing the connection is dangerous. The might be used from the caller after you closed it or get closed again.

Never free resources you do not own!

5

There’s the check() function:

    func check(e error){
	if e != nil {
		panic(e)
	}
}
6

And does it panic or does it run through? (remember that panics go on stderr and depending on how you run your programm might be logged into a different file or not appear at all!)

Have you tried logging before and after closing the connection?

Does it work when you replace ioutil.WriteFile with direct calls to os.Open, io.Write etc?

7

No, it does not
I’ve tried everything from the examples in The Tour of Go and nothing works
Logging after closing the connection does not help.

8

Logging after writing and error checking is not a question of if it works then, but a question of if the code is reached.

Please try to create a single go file with a mainpackage that reproduces your problem. Preferable is of course a repro that does not involve networking.

9

It’s just networking:


func main() {
    // Listen for incoming connections.
    l, err := net.Listen(CONN_TYPE, CONN_HOST+":"+CONN_PORT)
    if err != nil {
        fmt.Println("Error listening:", err.Error())
        os.Exit(1)
    }
    // Close the listener when the application closes.
    defer l.Close()
    fmt.Println("Listening on " + CONN_HOST + ":" + CONN_PORT)
    for {
        // Listen for an incoming connection.
        conn, err := l.Accept()
        if err != nil {
            fmt.Println("Error accepting: ", err.Error())
            os.Exit(1)
        }
        // Handle connections in a new goroutine.
        handleRequest(conn)
    }
}

// Handles incoming requests.
func handleRequest(conn net.Conn) {
  // Make a buffer to hold incoming data.
  buf := make([]byte, 1024)
  // Read the incoming connection into the buffer.
  size, err := conn.Read(buf)
  if err != nil {
    fmt.Println("Error reading:", err.Error())
  }
  s := string(buf[:size])
  fmt.Println(s)
  // Close the connection when you're done with it.
  conn.Close()
  perm := os.FileMode(0755)
  err = ioutil.WriteFile("output.txt", buf[:size], perm)
  check(err)
}

func check(e error){
	if e != nil {
		panic(e)
	}
}

took this from: https://coderwall.com/p/wohavg/creating-a-simple-tcp-server-in-go and just tried to write to file

10

Can you reproduce the faulty filewriting behaviour without all the networking stuff? If so, can you please share that shortened code?

11

I can’t
When i copy and paste the code, assigning buf manually(the assignment of buf in the server app is fine, since it transforms to s correctly) it works fine

12

Sorry, but I can not reproduce it locally with your snippet.

After I added the missing pieces (package, import (…), and const (…), I was able to compile and run.

echo "test" | nc localhost 3333 resulted created the expected output as well as it created the file, it contents was:

$ cat output.txt | xxd
00000000: 7465 7374 0a                             test.

Perhaps you are looking for the file in the wrong place and that one you are looking it is a leftover?

Please check the various dates a file in linux can have. Especially the mdate.

13

It does contain the string, but why doesn’t cat show the string, but xxd does?

14

What’s the string? If it doesn’t end with a newline it might get overwritten by your prompt, for example.

15

by string I mean what was in buf[:size] the question was: why don’t i see it with “cat output.txt” but can with “cat output.txt | xdd”?
And can I open it with another program(in C, for example)

16

What does the xxd version put out? It’s enough to tell us the last line. even the last two bytes are enough to maybe shed light on this.

17 
00000000: 6865 6c6c 6f0d                           hello.

“hello” is the string I tested it on

18

0x0d is decimal 13, decimal 13 is \r in ASCII, \r means to bring the cursor to the beginning of the same line.

So cat output.txt prints hello, moves the cursor back to where the h of hello were printed and then overwrites it with your shells prompt.

So to see if a file is empty, always check for its exact size in bytes: ls -l output.txt

2 Likes
19

Ok, thank you for your help