Working locally but failing when pushing to master #qq0

[serval-515]

I’m stuck on Stage #qq0
Implement the ECHO command

I’ve tried running the changes locally using this command -
echo -ne ‘*2\r\n$4\r\nECHO\r\n$6\r\nbanana\r\n’ | nc localhost 6379
It’s working, giving the expected output: ‘$6\r\nbanana\r\n’

When, I’m debug mode, I’m watching my output response variable, it’s coming correct and hence giving correct response on the terminal too. But, it’s failing when pushing to master

Here are my logs:

[tester::#QQ0] Running tests for Stage #QQ0 (Implement the ECHO command)
[tester::#QQ0] $ ./your_program.sh
[your_program] Server is running on port 6379
[tester::#QQ0] $ redis-cli ECHO raspberry
[tester::#QQ0] Sent bytes: "*2\r\n$4\r\nECHO\r\n$9\r\nraspberry\r\n"
[tester::#QQ0] Received: "" (no content received)
[tester::#QQ0]            ^ error
[tester::#QQ0] Error: Expected start of a new RESP2 value (either +, -, :, $ or *)
[tester::#QQ0] Test failed
[tester::#QQ0] Terminating program
[tester::#QQ0] Program terminated successfully

And here’s a snippet of my code:

// pasting the main logic, handling threading, reader and writer above this
try {
                    val command = reader.readText()
                    val commandList = command.split("\r\n")
                    val commandType = commandList[2]?.trim()?.uppercase()
                    var resp = ""
                    when (commandType) {
                        "PING" -> {
                            commandList.forEach {
                                if (it.trim().uppercase() == "PING") {
                                    resp += "+PONG\r\n"
                                }
                            }
                        }
                        "ECHO" -> {
                            val idxOfEcho = commandList.indexOf("ECHO")
                            for (i in idxOfEcho+1..<commandList.size) {
                                if (commandList[i] in listOf("", "\n", "\r\n")) continue
                                resp += "${commandList[i]}\r\n"
                            }
                        }
                        else -> {
                            resp = "-ERR unknown command '$commandType'\r\n"
                        }
                    }

                    writer.write(resp)
                    writer.flush()
}catch (e: Exception) {
                  println("Error: $e")
              } finally {
                  clientSocket.close()
              }

[andy1li]

The problem is caused by reader.readText(), which tries to read until the end of the stream. It’s easy to see that your local test ended quickly, but our test runner didn’t end because it’s waiting for your response.

Proof: I added flags before and after readText():

The after flag is not printed, proving the code hangs on readText().

Suggestion: Do not ignore the symbols and numbers – they can help you determine when to stop reading, thus preventing hanging.

*1\r\n$4\r\nPING\r\n

*2\r\n$4\r\nECHO\r\n$6\r\nbanana\r\n

[serval-515]

Hi Andy, actually I’m new to these challenges… could you explain more?? what do you mean by readText causing hanging, also can you deep dive a bit more on this,

but our test runner didn’t end because it’s waiting for your response.
Tests’s should be sending, one stream of input at once and then expecting the response right? Then, why it caused readText to hung up

[andy1li]

val clientSocket = serverSocket.accept()
val reader = BufferedReader(InputStreamReader(clientSocket.getInputStream()))

In this context, the stream comes from a socket connection. Only after the connection is closed will the stream actually end.

But after the tester sends you a command (echo foobar), it keeps the connection open and waits for your response. This means that there is no end for readText() at all, so it has to wait indefinitely.

Does this make more sense?

[serval-515]

Yes, Andy… I actually got the issue, so for local testing, I was running the server in debug mode and connecting and sending the ECHO command using Netcat. Netcat was sending the command, to my redis server and immediately closing the TCP connection, since readText was working fine but as per the above conversation, I think (Tests) they are using the same TCP connection for running all the tests, just correct my understanding, when the code runs for the first test, it opens up the connection to 6379, then sends the RESP command, and when it get’s the response back, then on the same connection it sends the next test RESP command, right?? After all tests are completed, then it finally closes the connection i believe.

I have changed my code, to this… using readLine instead of readText but now I think, it’s failing on some other test, but with same error
error:

remote: 
remote: [tester::#QQ0] Running tests for Stage #QQ0 (Implement the ECHO command)
remote: [tester::#QQ0] $ ./your_program.sh
remote: [your_program] Server is running on port 6379
remote: [tester::#QQ0] $ redis-cli ECHO blueberry
remote: [tester::#QQ0] Sent bytes: "*2\r\n$4\r\nECHO\r\n$9\r\nblueberry\r\n"
remote: [tester::#QQ0] Received: "" (no content received)
remote: [tester::#QQ0]            ^ error
remote: [tester::#QQ0] Error: Expected start of a new RESP2 value (either +, -, :, $ or *)
remote: [tester::#QQ0] Test failed
remote: [tester::#QQ0] Terminating program
remote: [tester::#QQ0] Program terminated successfully

code

fun main(args: Array<String>) {
     val serverSocket = ServerSocket(6379)
     serverSocket.reuseAddress = true
     println("Server is running on port 6379")

     while (true) {
          val clientSocket = serverSocket.accept()

            Thread {
                val reader = BufferedReader(InputStreamReader(clientSocket.getInputStream()))
                val writer = OutputStreamWriter(clientSocket.getOutputStream())

                try {
                    val commandList = parseInput(reader)
                    val commandType = commandList[2]?.trim()?.uppercase()
                    var resp = ""
                    when (commandType) {
                        "PING" -> {
                            commandList.forEach {
                                if (it.trim().uppercase() == "PING") {
                                    resp += "+PONG\r\n"
                                }
                            }
                        }
                        "ECHO" -> {
                            val idxOfEcho = commandList.indexOf("ECHO")
                            for (i in idxOfEcho+1..<commandList.size) {
                                if (commandList[i] in listOf("", "\n", "\r\n")) continue
                                resp += "${commandList[i]}\r\n"
                            }
                        }
                        else -> {
                            resp = "-ERR unknown command '$commandType'\r\n"
                        }
                    }

                    writer.write(resp)
                    writer.flush()
                } catch (e: Exception) {
                    println("Error: $e")
                } finally {
                    clientSocket.close()
                }
            }.start()
     }
}

fun parseInput(reader: BufferedReader): List<String> {
    val commandList = mutableListOf<String>()
    while(true) {
        val command = reader.readLine()
        if (command == null || command == "") break
        commandList.add(command)
    }
    return commandList
}

According to my understanding, since readLine reads till it finds some delimiter like \r or \n, then in this case if we don’t close the TCP connection, then also this should work meaning accepting the current command till it finds \r\n at end and “” empty at end of command, then passing this for processing. But, it’s not working as per my understanding
Btw, Thanks for the help Andy

[andy1li]

Yep, your understanding is completely correct!

Let’s look at the updated code:

while(true) {
    val command = reader.readLine()
    println(command)
    if (command == null || command == "") break
    commandList.add(command)
}

I added println to log out what readLine was able to read before the code hung.

1. It was able to read everything.
2. Before it hung, the code looked like readLine("").

till it finds “” empty at end of command # this is correct
then passing this for processing # but this is not

3. ReadLine won’t return until it can find a newline in its input, but there’s no newline in "" anymore. That’s why the code hung.

---

So the crux is to stop reading just when the content of the stream is exhausted.

One solution is to replace while(true) with a for loop, meaning you’ll need a stopping condition. This brings us back to the previous suggestion:

Do not ignore the symbols and numbers – you can use them to set the stopping condition for a for loop.

*1\r\n$4\r\nPING\r\n

*2\r\n$4\r\nECHO\r\n$6\r\nbanana\r\n

[serval-515]

Hello Andy, yes I got it… while(true) loop was causing the issue, since waiting for end of line, after exhausting the stream which it was not getting since the connection was not closed. And, causing the program to hang in there… Now I change this to FOR loop with stop condition, now it’s passing the current challenge tests, but started failing on the previous challenge tests… debugged from my side, but I’m not getting the issue.

Seeing, the logs it seems that client 1 established the TCP connection using redis-cli command, and sent PING for which he got the response as PONG first time, and we went ahead and close the connection right, running finally block. But, then again it’s sending the PING command on the same connection I believe? is it,

logs

remote: [tester::#QQ0] Running tests for Stage #QQ0 (Implement the ECHO command)
remote: [tester::#QQ0] $ ./your_program.sh
remote: [your_program] Server is running on port 6379
remote: [tester::#QQ0] $ redis-cli ECHO apple
remote: [tester::#QQ0] Sent bytes: "*2\r\n$4\r\nECHO\r\n$5\r\napple\r\n"
remote: [your_program] Received command: ECHO
remote: [your_program] Received command: apple
remote: [tester::#QQ0] Received bytes: "$5\r\napple\r\n"
remote: [tester::#QQ0] Received RESP bulk string: "apple"
remote: [tester::#QQ0] Received "apple"
remote: [tester::#QQ0] Test passed.
remote: [tester::#QQ0] Terminating program
remote: [tester::#QQ0] Program terminated successfully
remote: 
remote: [tester::#ZU2] Running tests for Stage #ZU2 (Handle concurrent clients)
remote: [tester::#ZU2] $ ./your_program.sh
remote: [your_program] Server is running on port 6379
remote: [tester::#ZU2] client-1: $ redis-cli PING
remote: [tester::#ZU2] client-1: Sent bytes: "*1\r\n$4\r\nPING\r\n"
remote: [your_program] Received command: PING
remote: [tester::#ZU2] client-1: Received bytes: "+PONG\r\n"
remote: [tester::#ZU2] client-1: Received RESP simple string: "PONG"
remote: [tester::#ZU2] Received "PONG"
remote: [tester::#ZU2] client-2: $ redis-cli PING
remote: [tester::#ZU2] client-2: Sent bytes: "*1\r\n$4\r\nPING\r\n"
remote: [your_program] Received command: PING
remote: [tester::#ZU2] client-2: Received bytes: "+PONG\r\n"
remote: [tester::#ZU2] client-2: Received RESP simple string: "PONG"
remote: [tester::#ZU2] Received "PONG"
remote: [tester::#ZU2] client-1: > PING
remote: [tester::#ZU2] client-1: Sent bytes: "*1\r\n$4\r\nPING\r\n"
remote: [tester::#ZU2] Received: "" (no content received)
remote: [tester::#ZU2]            ^ error
remote: [tester::#ZU2] Error: Expected start of a new RESP2 value (either +, -, :, $ or *)
remote: [tester::#ZU2] Test failed
remote: [tester::#ZU2] Terminating program
remote: [tester::#ZU2] Program terminated successfully

and updated parseInput code

fun parseInput(reader: BufferedReader): List<String> {
    val commandList = mutableListOf<String>()
    val numberOfCommands = reader.readLine() // will give, *2, *3, etc
    val numberOfCommandsInt = numberOfCommands?.substring(1)?.toInt() ?: 0

    for (i in 0 until numberOfCommandsInt) {
        reader.readLine()
        val command = reader.readLine()
        println("command: $command")
        commandList.add(command)
    }

    return commandList
}

[andy1li]

Nice job implementing the for loop!

Seeing, the logs it seems that client 1 established the TCP connection using redis-cli command, and sent PING for which he got the response as PONG first time, and we went ahead and close the connection right, running finally block.

No, we didn’t close the connection. After handling the first ping, we just stopped reading any commands.

We needed to keep reading, after we responded to a command.

But, then again it’s sending the PING command on the same connection I believe? is it,

Yep, you’re right.

---

Suggestion: refactor your code around an outer while loop like this:

while (true) {
    [try reading from the stream]

    [parse one command]

    [respond to the command]
}

[serval-515]

Yeah, Thanks, Andy, for the hints. I really appreciate them. I passed this challenge. Actually, I wasn’t taking the case where a single client can send multiple commands to the same connection and hence after responding to single command, was closing the client connection by running finally block. But yeah, interesting discussion!