dennis-tra / nebula

🌌 A network agnostic DHT crawler, monitor, and measurement tool that exposes timely information about DHT networks.
Apache License 2.0
294 stars 30 forks source link

fix: crawl_error detection #57

Closed guillaumemichel closed 7 months ago

guillaumemichel commented 7 months ago

If all runs of the loop failed, then ErrorBits is a power of two minus one, and we should keep the result.Error. Otherwise, at least one iteration was successful, set result.Error to nil.

dennis-tra commented 7 months ago

To be on the very safe side, could you extract a function like:

func hasAtLeastOneSuccesfulFindNodeOrBetterAShorterFunctionName(errorBits uint32) bool {
   ...
}

and write a test that passes a few reasonable values into it and asserts that it works correctly?

This would also serve as documentation how the logic is supposed to work

dennis-tra commented 7 months ago

Great thanks, let's merge!

Just want to share my thoughts. I initially thought of a test that was more focussing on the behaviour than the logic (subtle difference). Something along these lines:

    tests := []struct {
        name      string
        err       error
        errorBits uint32
        want      bool
    }{
        {
            name:      "no err",
            err:       nil,
            errorBits: 0b00000000,
            want:      false,
        },
        {
            name:      "first failed",
            err:       fmt.Errorf("some err"),
            errorBits: 0b00000001,
            want:      true,
        },
        {
            name:      "second failed, first worked",
            err:       fmt.Errorf("some err"),
            errorBits: 0b00000010,
            want:      false,
        },
    }

but this test is totally fine 👍

guillaumemichel commented 7 months ago

added the tests as you suggested