search

google / certificate-transparency-go

Auditing for TLS certificates (Go code)
https://certificate.transparency.dev
Apache License 2.0
894 stars 232 forks source link

Retriable errors and backoffs #898

Open jsha opened 2 years ago

jsha commented 2 years ago

I'm working on a tool that uses fetcher.go, and also calls GetProofByHash. I'd like to configure it to do backoffs and retries. I see that fetcher.go uses a Backoff struct provided internally:

https://github.com/google/certificate-transparency-go/blob/6ecca400d5e42ae3f26ad5c27cea9f907474024d/scanner/fetcher.go#L273-L292

But I think that usage might be broken. Backoff will only retry on grpc error codes that are retriable, plus the RetriableError struct. The GetRawEntries call I linked to is implemented by jsonclient.GetAndParse, which returns RspErrors. Those RspErrors don't match any of the conditions for retries.

I'd like to propose this: In addition to / instead of backoff.RetriableError, the backoff package should check for some interface, e.g. Retryable() bool. Then jsonclient.RspError can implement Retryable() as: a RspError is retryable if its wrapped error is a net.Timeout error OR the HTTP status code is >500.

What do you think?

pav-kv commented 2 years ago

Good catch, I think you are right. The retry will still happen because of the continue in line 289, but there will be no backoff in this case.