Bug 2049856 - Remove old viaduct backend code [ci full]#7450
Conversation
| #[cfg(target_os = "ios")] | ||
| timeout: 7000, | ||
| #[cfg(not(target_os = "ios"))] | ||
| timeout: 10000, |
There was a problem hiding this comment.
From markh in an earlier version:
I don't think these make sense - I've lost some of the history, but I don't think the very first cuts of viaduct had such short timeouts. The problem is that none of us will ever see a problem as we are on good networks, but things just randomly break in the field - ie, the 10s sync timeout was introduced (I'm quite sure we never started with that timeout), but it took some time for us to finally notice in user-generated bug reports that it was a problem.
I'd rather invert this - have sane defaults and individual clients who really do need insanely short timeouts get to override?
There was a problem hiding this comment.
Thanks for catching this, I bumped the timeout back up to 60s.
I got confused because this was the defaults for the old backend that I was removing: https://github.com/mozilla/application-services/pull/7449/changes#diff-f05535f2df59d33408a4bec415bf621aa648501e7f6f5279424e7d74684636c7L39. However, I guess that wasn't actually taking effect because all platforms are using the new backend now, which is why we can open this PR in the first place.
The thing I still don't understand why we weren't seeing timeouts before we switched to the new backend. With the old backend the defaults did seem to be 7s and 10s and I don't see how they were overridden.
59f916d to
dd1f60a
Compare
dd1f60a to
ccbdf5e
Compare
* Removed the old `backend` and `backend::ffi` modules * Renamed `new_backend` to `backend`. Changed the default timeout for `viaduct::Client` to 7 seconds for iOS and 10 seconds otherwise. This matches the behavior of we had for `Request::send()`. Should we make these longer? Also: * Fixed build errors when crates depend on viaduct with the ohttp feature, but not rusqlite. * removed viaduct-reqwest which was no longer used. * Removed un-used protobuf code.
ccbdf5e to
f515016
Compare
backendandbackend::ffimodulesnew_backendtobackend.Changed the default timeout for
viaduct::Clientto 7 seconds for iOS and 10 seconds otherwise. This matches the behavior of we had forRequest::send(). Should we make these longer?Also: removed viaduct-reqwest which was no longer used. Removed the
rusqlite::bundledfeature fromads-client. It's not set by other crates and this fixed a build issue for me.Pull Request checklist
[ci full]to the PR title.