fix(node): commit http errors in data request retrieval

[tmpolaczyk]

Fix #2306

[aesedepece]

This change sounds like a good candidate for being shipped with 1.6 — behind a TAPI flag if necessary. What do you think?

[tmpolaczyk]

I believe this can be merged without TAPI, because the old behavior is to not commit anything at all. Therefore the data requests will be unsolved until a majority of miners update, and then it will just work as expected.

It is true that we won't have a guarantee that these data requests will be solved until the next TAPI update, which ensures that miners have updated, but I believe nothing will break if this change is merged without TAPI.

[aesedepece]

It is true that we won't have a guarantee that these data requests will be solved until the next TAPI update, which ensures that miners have updated, but I believe nothing will break if this change is merged without TAPI.

I'm just fine with that approach. Thanks!

[guidiaz]

In relation to what was discussed in #2050, I'd suggest to let a single HTTP/403 error in any of the data request's sources be interpreted as a sufficient condition to make the witness node not to commit to such data request. This way we'd make data requests relying on private APIs to be systematically resolved with InsufficientCommits, unless there was a minimal amount of witnessing nodes prepared to attend involved private API. El mar, 10 ene 2023 a las 16:44, aesedepece ***@***.***>) escribió:

…

It is true that we won't have a guarantee that these data requests will be solved until the next TAPI update, which ensures that miners have updated, but I believe nothing will break if this change is merged without TAPI. I'm just fine with that approach. Thanks! — Reply to this email directly, view it on GitHub <#2307 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AT6FV4UAUJECKHI5XXRNSZTWRV7XTANCNFSM6AAAAAASBCR2EY> . You are receiving this because you commented.Message ID: ***@***.***>

[tmpolaczyk]

I don't think it is a good idea to special-case HTTP/403 errors, because many APIs return 404 or 401 instead.

[guidiaz]

I don't think it is a good idea to special-case HTTP/403 errors, because many APIs return 404 or 401 instead.

Yeah right, either HTTP/401 or HTTP/403 errors in one single source would be sufficient condition for not commiting to a data request. Data requests relying on a majority of private API sources returning 404 error codes would just be resolved as HttpError { code: 404 }.

[aesedepece]

@tmpolaczyk shall we rebase and merge this onto 1.6?

[tmpolaczyk]

@tmpolaczyk shall we rebase and merge this onto 1.6?

Done