| Commit message (Collapse) | Author | Age | Files | Lines |
| | |
|
| | |
|
| | |
|
| |
|
|
| |
cargo fmt, precisely.
|
| | |
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
I identified the cases to replace by searching for the string
`.report()`. There are a few that I didn't change:
* A couple of cases that used anyhow::Error,
* One case that reported two Errors.
* Two cases in `tor_hsclient::err` that just did
`error!("Bug: {}")`.
I have also not audited the cases in `tor-hsclient` where we're using
`tor_error::Report` manually.
Nonetheless, closes #949.
|
| |
|
|
|
|
|
| |
My goal here is to make sure that we can't confuse
one download operation and another, and that we actually know
what's going on. Previously, not all state transitions or
attempts to fetch information actually corresponded to a log.
|
| | |
|
| | |
|
| |
|
|
|
| |
This panics on error, and we're fine with a panic on misbehavior in
tests.
|
| |
|
|
|
| |
This is precisely the result of running the rune in
maint/adhoc-add-lint-blocks.
|
| |
|
|
| |
This will allow us to get rid of a needless trait impl.
|
| |
|
|
|
|
|
| |
We're going to have functions on Response that fail by returning only
one of these.
Sadly this diff is quite noisy.
|
| |
|
|
|
| |
These don't check errors and are therefore quite hazardous.
I'm going to introduce a more cooked version in a moment.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This fixes an busy-loop.
When the last `TaskHandle` on a `TaskSchedule` is dropped, the
schedule is permanently canceled: whatever operation it was
scheduling should no longer be performed. But our code was broken:
the `sleep()` and `sleep_until_wallclock()` functions don't verify
whether the handles are dropped or not.
This breakage caused an CPU-eating busy-loop in
`sleep_until_wallclock`.
With this patch, we now return a `Result<(), SleepError>` from these
functions.
Fixes #572.
|
| |
|
|
| |
Its existence tended to hide bugs, and was just asking for trouble.
|
| | |
|
| |
|
|
|
| |
If there was no partial information, then calling update_progress
will now report a stall in this case, which is not what we want.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
A "reset" happens whenever we have to start a download attempt over
-- either because we ran out of retries, or we found something wrong
with the consensus after fetching certificates.
An "error" happens when we have a recoverable error from one or more
directory sources.
A "stall" happens whenever a round of downloads or cache loads leads
to no change in the status.
We don't yet use this as part of our status reporting.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
Previously we used the "if-modified-since" time associated with the
consensus download, and/or the "valid-after" time in the consensus
attempt, to put multiple attempts into sequence, and to tell one
from another. But that approach was always a kludge, and will soon
get more unreliable as the DirStatus logic gets a bit more complex.
With this commit, we change separate download attempts to be
identified with an AttemptId that increments whenever we decide to
get a different directory from the one we have. IMO this new code
is _much_ cleaner.
|
| |
|
|
| |
This is all crate-internal APIs, fortunately.
|
| |
|
|
|
|
| |
This change (not yet exposed as an API) will let the TorClient have
a `TaskHandle` corresponding to the directory task, letting it
make the directory task dormant as needed.
|
| |
|
|
|
|
|
|
| |
Now that the relevant functions now report changed/not-changed
status via a boolean out-parameter (see !527), there's no reason to
have a separate NoChanged error case.
Closes #484.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
Previously in !511 I had introduced a bug where, if there was an
error more serious than "no change", that error would keep us from
noticing that we had no change, and we'd loop until the safety
counter ran out. Then we'd panic.
This commit fixes the bug by reintroducing the `changed` boolean --
this time as an outparam for the add_from_* methods.
Fixes #482.
|
| | |
|
| | |
|
| |
|
|
| |
It does nothing that Fatal does not. Suggested by @eta in review.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We no longer have separate return paths for recoverable and fatal
errors; instead, they are merged, and distinguished based on
recovery actions.
Since it is now possible for download() to give an error that should
_not_ destroy the previous state, it takes `&mut Box<dyn DirState>`.
This change unfortunately means that we can no longer call `state =
state.advance()`, but instead have to do some mem::swap junk with
poisoned values. Any better solution would be a good thing.
Additionally, the reset() and advance() methods can no longer fail.
There is still a separate return path for reset-triggering errors;
I'm about to fix that.
|
| | |
|
| | |
|
| |
|
|
|
| |
Fortunately, the only error type that we need to handle blocking
errors with actually has a DirSource in it already.
|
| |
|
|
|
| |
(A blocking error is one that means that the current bootstrap
attempt has failed, and must be restarted.)
|
| |
|
|
|
|
|
| |
Previously all errors were treated as non-fatal.
The add_from_* implementations don't yet behave properly;
I'll fix them in subsequent commits.
|
| |
|
|
|
|
|
|
|
| |
Previously DocSource would tell you whether the document was from
a local store or a cache server, but it wouldn't tell you _which_
server it came from.
This change required adding DocSource as an argument to
DirState::add_from_download.
|
| |
|
|
|
|
|
|
|
|
| |
With this API we can now stop consensus download attempts early if
any consensus that the directory cache gave us would be necessarily
too far in the future or in the past.
This saves wasted bandwidth for clients with skewed clocks.
Closes #466.
|
| |
|
|
|
|
|
|
|
|
| |
If we're happy with a directory from 3 days ago, we should say
"if-modified-since 3 days ago".
This patch is larger than I'd like, since I had to add &DirMgrConfig
as an argument to the functions that make a consensus request.
Closes #467.
|
| | |
|
| |
|
|
|
|
|
| |
- Some FIXMEs got removed or amended.
- AddMicrodescs now yields a mutable reference, so we can use .drain()
and reuse the allocation.
- Some panics were downgraded to debug_asserts.
|
| |
|
|
|
|
|
|
| |
- We don't want to inadvertently replace our netdir with one that's
actually older, so detect and error on this condition.
- Also, print a debug line when we get a new netdir without enough
guards.
- (An unrelated TODO was also added.)
|
| |
|
|
|
|
|
|
|
|
|
| |
- GetMicrodescsState now uses the NetDirChange API to propagate netdir
changes, instead of modifying the netdir directly.
- PendingNetDir was refactored in order to support this use case.
- As a result, the netdir-related methods in WriteNetDir can be removed,
leaving only the DirFilter for now.
- add_from_cache() no longer takes a store, because nothing uses it.
- (bodge: apply_netdir_changes() was put in a few places missed
previously)
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
- The new DirState::get_netdir_change() API lets the state machine
export a NetDirChange: a request to either replace the current netdir,
or add microdescs to it.
- bootstrap.rs now consumes this new API, even though nothing implements
it yet.
- This will let us implement GetMicrodescsState without having to
directly mutate the netdir. The calling code also handles checking the
netdir against the circmgr for sufficiency, and updating the consensus
metadata in the store, meaning the revised GetMicrodescsState will not
have to perform these tasks.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
- The additional parameters passed to GetConsensusState are now passed
through all the states, and used as well.
- WriteNetDir doesn't have a now() or config() method any more, since
the states now get this from the runtime or the config parameters.
- This required modifying the tests to make a mocked runtime and custom
config directly, instead of using DirRcv for this purpose.
- Additionally, because we don't have to upgrade a weak reference for
DirState::dl_config(), that function no longer wraps its return value
in Result.
- (A bunch of the FIXMEs from the previous commit that introduced the
additional parameters have now been rectified as a result.)
|
| |
|
|
|
|
| |
- Given that this is effectively an implementation detail, it doesn't
really make sense to have it be in the crate root...
- (also, we're going to change it a bunch now)
|
| |
|
|
|
|
|
| |
- fetch_single now takes what it needs, instead of an Arc<DirMgr<R>>.
- This required refactoring the CANNED_RESPONSE mechanism, given the
test would otherwise fail due to not having a CircMgr to pass to
fetch_single.
|
| |
|
|
|
| |
- DirMgr::note_request_outcome and friends are now just standalone
functions, taking a CircMgr.
|
| |
|
|
|
|
|
| |
- query_into_requests is now called make_requests_for_documents, and
does the &[DocId] -> DocQuery conversion internally instead.
- DirMgr::make_consensus_request and DirMgr::query_into_requests are now
gone. The tests use the new functions, as does fetch_multiple.
|