Opened 9 months ago

Closed 8 months ago

#28870 closed defect (fixed)

Stop asserting when there's not a descriptor for a relay being measured

Reported by: juga Owned by:
Priority: Medium Milestone: sbws: 1.0.x-final
Component: Core Tor/sbws Version: sbws: 1.0.2
Severity: Normal Keywords:
Cc: juga, teor Actual Points:
Parent ID: #28663 Points:
Reviewer: asn Sponsor:

Description

When sbws doesn't have a descriptor for a relay that is being measured (but it has a network status document, and possibly the other way around), it raises and exception via this assert (https://github.com/torproject/sbws/blob/ee64d76df54ceb3a3c9e1e2a797fd70d68bb0035/sbws/lib/relaylist.py#L45), which causes the error callback in the thread pool in scanner.
Instead, it should just return None (what can also be done by getattr and removes redundant code), and all the logic that depends on it will just work fine, it just won't be choose as an exit, since there's no information about the exit flags, but will get measured.

Child Tickets

Change History (6)

comment:1 Changed 9 months ago by juga

Status: newneeds_review

comment:2 Changed 9 months ago by juga

Status: needs_reviewneeds_revision

i realized there's a bug in the PR

comment:3 Changed 9 months ago by juga

Status: needs_revisionneeds_review

Add one more commit

comment:4 Changed 9 months ago by dgoulet

Reviewer: asn

comment:5 Changed 8 months ago by asn

Status: needs_reviewmerge_ready

ac9406a LGTM.

02bb2f5 also LGTM, but as a nitpick IMO it's a bit sneaky to call this a 'rename' since a rename implies that there is no behavior change, whereas that commit actually fixes a bug IIUC. I guess this bug went undetected due to the lack of strong typing in Python and no unittests walking over it.

comment:6 Changed 8 months ago by juga

Resolution: fixed
Status: merge_readyclosed

Thanks!. Reworded 02bb2f5 message and merged.

Note: See TracTickets for help on using tickets.