Skip to content
This repository was archived by the owner on Nov 20, 2025. It is now read-only.

Conversation

@JustinBeckwith
Copy link
Contributor

Fixes #606. In most cases, this library would throw if an attempt to refresh a token was made, and no refresh token was available. It turns out there was a loophole here if the getRequestHeaders method was called, and the throw would get lost in the shuffle. Instead, a call to the OAuth2 refresh endpoint would get made with an empty refresh token, leading to a less awesome exception. This change:

  • moves the place where we throw
  • adds a test to verify the behavior
  • Adds a describe block in the test file

Make sure to hide whitespace before reviewing this one :)

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 13, 2019
@codecov
Copy link

codecov bot commented Apr 13, 2019

Codecov Report

Merging #670 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #670   +/-   ##
=======================================
  Coverage   88.29%   88.29%           
=======================================
  Files          18       18           
  Lines         769      769           
  Branches       83       83           
=======================================
  Hits          679      679           
  Misses         79       79           
  Partials       11       11
Impacted Files Coverage Δ
src/auth/oauth2client.ts 86.45% <100%> (ø) ⬆️

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 dffd1cc...d3b4bf3. Read the comment docs.

@codecov
Copy link

codecov bot commented Apr 13, 2019

Codecov Report

Merging #670 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #670   +/-   ##
=======================================
  Coverage   88.29%   88.29%           
=======================================
  Files          18       18           
  Lines         769      769           
  Branches       83       83           
=======================================
  Hits          679      679           
  Misses         79       79           
  Partials       11       11
Impacted Files Coverage Δ
src/auth/oauth2client.ts 86.45% <100%> (ø) ⬆️

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 8cc5522...2637c7e. Read the comment docs.

Copy link
Contributor

@alexander-fenster alexander-fenster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rubber stamp even though I don't fully understand how it works :(

@JustinBeckwith JustinBeckwith merged commit 0a02946 into master Apr 16, 2019
@alexander-fenster alexander-fenster deleted the rtt branch May 8, 2019 19:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a test to cover #605

3 participants