[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #31794 [Circumvention/Snowflake]: Errors swallowed

#31794: Errors swallowed
-------------------------------------+------------------------
 Reporter:  sah                      |          Owner:  (none)
     Type:  defect                   |         Status:  new
 Priority:  Medium                   |      Milestone:
Component:  Circumvention/Snowflake  |        Version:
 Severity:  Normal                   |     Resolution:
 Keywords:                           |  Actual Points:
Parent ID:                           |         Points:
 Reviewer:                           |        Sponsor:
-------------------------------------+------------------------

Comment (by cohosh):

 Thank you for looking into this sah.

 Instead of replying on how each individual error should be handled, I'm
 going to give a few examples and save an individual analysis on whether
 that was the right call for a code review. A lot of this will depend on
 context, so whoever takes the ticket is welcome to apply their best
 judgement and get feedback from the reviewer.

 - If according to your judgement or according to a previous call to the
 function, the calling function to take some action or behave differently
 on an error, we should do so. An example of this is:
 {{{
  broker/broker.go:372:34: Error return value of
 ctx.metrics.LoadGeoipDatabases is not checked (errcheck)

     ctx.metrics.LoadGeoipDatabases(geoipDatabase, geoip6Database)
 }}}
  Earlier in the code, the same function causes a `log.Fatal` to be thrown
 on error and it should here too:
 {{{
  err := ctx.metrics.LoadGeoipDatabases(geoipDatabase, geoip6Database)
  if err != nil {
     log.Fatal(err.Error())
  }
 }}}
 - If the surrounding code should keep doing what it's doing if it
 encounters the error, then log it or ignore it depending on how useful
 that will be for debugging purposes. An example of this is:
 {{{

 broker/broker.go:186:9: Error return value of w.Write is not checked
 (errcheck)

     w.Write(offer)


 broker/broker.go:220:10: Error return value of w.Write is not checked
 (errcheck)

     w.Write(answer)
 }}}
  We don't want the broker behaviour to change, we still want it to listen
 for proxy polls. I think logging an error makes sense here so we can
 detect an unusual amount of proxy connection errors, but we might change
 our minds if it's too noisy.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/31794#comment:2>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online

_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs