404 - Not Found - Issue 19#82
Conversation
…code and path when file not found
|
I did miss adding tests for the code I added in ClientHandler, including logging and the if else. I will be working on that for a bit. Edit: Nevermind. I will wait for comments first as a ClientHandlerTest is already in the works. |
joejoh84
left a comment
There was a problem hiding this comment.
Looks good, but i am unsure about expected behavior when 404 error page is not present in webroot. Should the server write out default message here? now it just get stuck in loading. I believe its due to ClientHandler trying write response[1]
FileRequestHandler:
public byte[][] writeResponse(String responseCode) { String response = "HTTP/1.1 " + responseCode + " \r\nConnection: Closed\r\n\r\n"; return new byte[][]{response.getBytes()}; }
ClientHandler:
try { fileInfo = fileRequestHandler.handleRequest(input); response = fileRequestHandler.writeResponse(fileInfo); } catch (FileNotFoundException e){ LOGGER.error("File not found: {}", e.getRequestPath()); response = getFileNotFoundResponse(fileRequestHandler, e); } out.write(response[0]); out.write(response[1]);
FelixJacobsen
left a comment
There was a problem hiding this comment.
The code looks good, I like the usage of a Enum as "Response.Status" is defined.
I agree with @joejoh84, there should be some kind of default message to prevent it getting stuck in loading. But the code should do what it is supoposed to do and solve the issue.
If a file/url is not found, an exception created and is logged.
Definitely can use a bit of refactoring here and there.