openid / OpenYOLO-Web

Web protocol for credential exchange and update - "You Only Login Once"
http://openid.net/wg/ac/
Apache License 2.0
100 stars 16 forks source link

Better timeout handling, and allow to set custom timeout #37

Closed TMSCH closed 7 years ago

TMSCH commented 7 years ago

Refactored the way timeout are handled such that we can ensure every request (hint, retrieve, etc.) actually times out after the expected value (3sec for instance, or user defined) instead of having many separate operations that can time out, which actually create a global upper bound timeout longer than expected (i.e. secure channel connection timeout length + wrap browser request timeout + actual request timeout).

Introduced for this the TimeoutRacer, which is an object that can be passed along methods, such that the timeout duration remains global and at any point, the flow can stop if it expires. This allows for granular control of the flow and to properly dispose of pending operations.

Extracted the browser implementation of OpenYolo in its own class for cleaner design.

More work can be done, especially I'd like to break down the api.ts file so that we can write more targeted tests. Currently the api_test.ts test cases are between unit and functional tests and are not very reliable as a lot of logic is untested.

codecov-io commented 7 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@2b15ace). Click here to learn what that means. The diff coverage is 85.91%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #37   +/-   ##
=========================================
  Coverage          ?   88.28%           
=========================================
  Files             ?       34           
  Lines             ?     1374           
  Branches          ?      161           
=========================================
  Hits              ?     1213           
  Misses            ?      161           
  Partials          ?        0
Impacted Files Coverage Δ
ts/api/wrap_browser_request.ts 37.5% <ø> (ø)
ts/api/credential_request.ts 100% <ø> (ø)
ts/api/cancel_last_operation_request.ts 100% <ø> (ø)
ts/api/disable_auto_sign_in.ts 100% <ø> (ø)
ts/api/hint_request.ts 100% <ø> (ø)
ts/api/hint_available_request.ts 100% <ø> (ø)
ts/api/base_request.ts 98.5% <100%> (ø)
ts/api/api.ts 83.06% <82.79%> (ø)
ts/protocol/utils.ts 82.4% <87.5%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 2b15ace...1d6f143. Read the comment docs.

TMSCH commented 7 years ago

Thanks Rahul!