Blobber returns 400 with misleading error message when it fails to gain sharder consensus

[stewartie4]

When processing a commit request at v1/connection/commit/ the blobber attempts to get the write pool from the sharders. a consensus of 50% is required.
If this consensus is not reached (for example one or multiple sharders returning a 503) the blobber returns the following response:
HTTP 400
{"code":"write_pre_redeem","error":"write_pre_redeem: can't request write pools from sharders: requesting write pools stat: invalid_response: Sharder responses were invalid. Hash mismatch"}

There are a few issues with this that could hopefully be resolved to make a more resilient API:

1. HTTP 503 is a retryable status code. Rather than the blobber immediately excluding a sharder in a 503 scenario, it should inspect the response code and if a 503, wait a few seconds and retry the request. This will resolve the issue the majority of the time
2. The blobber returns the response "Sharder responses were invalid. Hash mismatch" which is misleading as the issue is really with connectivity (this message will create a lot of confusion for developers who will assume there is a data integrity issue). In fact this message is a generic catch all for "there was an issue with the sharder response" - could the error messages be made more specific eg. Include the sharder response?
3. The response code from the blobber to the client is 400 Bad request. This suggests an issue with the client's request that needs to be fixed on their side. In reality this is an environmental issue with the network and the request can be retried as-is.
   Clients often use http response status codes for error control flow and 4xx is unlikely to be retried. Therefore in keeping with REST standards the status code from the blobber should be 5xx (our fault,retry) not 4xx (your fault don't retry).

[ronaudinho]

from what I've gathered:

1. this is doable, though I'm not that familiar with the sharder yet, so pinning retries on HTTP response code assumes that sharder's response conforms with REST standards as well (see point 3). I'll take a look at the repo and get back to you.
2. what do you think about informing the client if consensus may not be reached as there are non-2xx responses?
   i.e when there are 3 sharders, 2 responds with 200 but different hash, 1 responds with non-2xx, something like

invalid_response: hash mismatch: some sharders are not available

but when all 3 responds with 200 and 50% consensus is not reached, simply says

invalid_response: hash mismatch

3. the current handler responds with 400 for any error, so this change will require a bit of deliberation.

[guruhubb]

yes this makes sense to add some specific errors.

[stewartie4]

@ronaudinho Agreed, hopefully a 503 will be a good place to start as we know it is definitely returned in some instances.
Granular error codes/messages are always a win, anything to make it easier for the client to recover/the dev to integrate is good in my book.

On 3. - that's unfortunate, that sounds like it's big enough to be another feature of it's own - adding an error classifier/HTTP code mapper.
As you have mentioned, a lot of this is caused by REST standards not being followed so any small improvements would be great and build up over time

[ronaudinho]

given 4 sharders, how many should come into consensus for the response to be valid? 2 or 3 of them?

what if there are only 2 sharders? do we need both to agree?

[stewartie4]

looks to currently be (numSharders / 2) 50%

[ronaudinho]

looks to currently be (numSharders / 2) 50%

yeah but based on these lines, if the sharder is down, it will be excluded, so if maxCount <= (numSharders / 2) does not make sense to me:

1. 0 sharder should return an not enough sharder available or no sharder available error?
2. 1 sharder should return an not enough sharder available error as well? (there is no consensus to reach if there is only 1 response, right?)
3. if there are only 2 sharders, I believe it will have to be 100% consensus
4. when the number of available sharders are more than 2 and they are even, a 50% + 1 consensus is required, which is what I wanted to confirm (e.g. currently with 4 available sharders, if 2 of them are in consensus, the call will still return a hash mismatch)

I tried tracking down the HTTP response code and only found 503 if it goes through sdkproxy, the rest should all return 400 (or 500 if rate limit is hit), but CMIIW.

anyway, I'll try to PR a fix for this by today.

[guruhubb]

1. Your suggestion is better.
2. Not if there is only 1 sharder in the blockchain. Then its fine and 100% consensus.
3. One is fine, because client can always verify the block from the sharder and subsequent blocks to build enough length for deterministic finality of a block. Also, one sharder could be down.
4. Same as 3. 50% is fine because client can verify. Also it depends how many sharders you query. If there are 10 sharders and as a client you query 2, then 50% response is fine. There isn't "consensus" that we need here. Consensus of a transaction is done by the miners and is a different process.

[andrenerd]

Added extra "calls" to temporarily unavailable sharders (if 503/504 error is returned).

The resulting error message and status are not changed. I guess there is no reason to change them. If shader(s) doesn't reply after several attempts, it should be assumed to be "failed" and all the further actions, including error reporting, are legit.

PR #79