FTP: add more detailed error codes based on RFC 959.

FTP: add more detailed error codes based on RFC 959.

This does not handle all cases yet, for simplicity.

TEST=net_unittests
BUG=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=52495

[Paweł Hajdan Jr., 2010-07-14 21:53:56]

Hope you like it! No more ERR_FAILEDs in ftp_network_transaction.cc.

[eroman, 2010-07-14 22:35:07]

LGTM.

I am very supportive of having non-ERR_FAILED codes :)

http://codereview.chromium.org/2985012/diff/1/2
File net/base/net_error_list.h (right):

http://codereview.chromium.org/2985012/diff/1/2#newcode394
net/base/net_error_list.h:394: NET_ERROR(FTP_COMMAND_FAILED, -601)
I suggest using the prefix: FTP_* rather than FTP_COMMAND_* throughout.

http://codereview.chromium.org/2985012/diff/1/3
File net/ftp/ftp_network_transaction.cc (right):

http://codereview.chromium.org/2985012/diff/1/3#newcode88
net/ftp/ftp_network_transaction.cc:88: NOTREACHED();
nit: might be useful to add

<< response_code

http://codereview.chromium.org/2985012/diff/1/3#newcode721
net/ftp/ftp_network_transaction.cc:721: case ERROR_CLASS_TRANSIENT_ERROR:
For debugging purposes, I would also recommend adding the raw error codes into
the NetLog. (However that is a separate unrelated change).

http://codereview.chromium.org/2985012/diff/1/5
File net/ftp/ftp_network_transaction_unittest.cc (left):

http://codereview.chromium.org/2985012/diff/1/5#oldcode1224
net/ftp/ftp_network_transaction_unittest.cc:1224: "500 failed list\r\n",
What is the motivation for changing these 500s to 599s?

[Paweł Hajdan Jr., 2010-07-15 01:56:02]

http://codereview.chromium.org/2985012/diff/1/2
File net/base/net_error_list.h (right):

http://codereview.chromium.org/2985012/diff/1/2#newcode394
net/base/net_error_list.h:394: NET_ERROR(FTP_COMMAND_FAILED, -601)
On 2010/07/14 22:35:07, eroman wrote:
> I suggest using the prefix: FTP_* rather than FTP_COMMAND_* throughout.

Done.

http://codereview.chromium.org/2985012/diff/1/3
File net/ftp/ftp_network_transaction.cc (right):

http://codereview.chromium.org/2985012/diff/1/3#newcode88
net/ftp/ftp_network_transaction.cc:88: NOTREACHED();
On 2010/07/14 22:35:07, eroman wrote:
> nit: might be useful to add
> 
> << response_code

Done.

http://codereview.chromium.org/2985012/diff/1/5
File net/ftp/ftp_network_transaction_unittest.cc (left):

http://codereview.chromium.org/2985012/diff/1/5#oldcode1224
net/ftp/ftp_network_transaction_unittest.cc:1224: "500 failed list\r\n",
On 2010/07/14 22:35:07, eroman wrote:
> What is the motivation for changing these 500s to 599s?

Now 500 means a syntax error. I think that in real world these commands are very
unlikely to fail with that error code, so I've used an "unallocated" FTP error
code.

[eroman, 2010-07-15 01:59:19]

lgtm

[wtc, 2010-07-20 18:37:09]

LGTM!  Thanks.

http://codereview.chromium.org/2985012/diff/6001/7004
File net/ftp/ftp_network_transaction_unittest.cc (right):

http://codereview.chromium.org/2985012/diff/6001/7004#newcode85
net/ftp/ftp_network_transaction_unittest.cc:85: "599 fail\r\n");
Just curious: what do you use the "599" response status code for in unit tests?

Ah, I see that eroman asked the same question.  So your use of 599 is worth
a comment!