Apply the person token refresh to the request that triggered it

[eastagiletracker]

This PR proposes applying the person token refresh to the request that triggered it, so a call made after the access token has expired goes out with the refreshed token instead of the expired one. We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/334. You can sign in with your GitHub ID to claim ownership of the project.

What's wrong

BearerTokenAPI.request() builds the outgoing Request object — including the Authorization header, read straight off this.token_set.access_token — and only afterwards checks whether the person token set needs refreshing. By the time the refresh runs and writes the new token back onto this.token_set, req has already captured the old header, so fetch(req) still sends the expired access token and the call fails with an invalid-access-token error. The refreshed token only takes effect on the next request.

That is the opposite of what the SDK promises. The note on PersonTokenSet in src/types/common.ts reads "If you do not provide a refresh token the SDK will not be able to refresh the token for you and requests made an hour after the initial token exchange will fail" — i.e. supplying a refresh_token is supposed to keep requests working past the token lifetime. Today it does not: the first request after expiry fails anyway, and only a manual retry succeeds. Every Graph, User and Audit call goes through this method, so it affects any integration using person token sets. Introduced in d4225db (reset the api file changes from prettier), where the refresh block ended up below the Request construction.

The change

The refresh check moves ahead of the header assignment, so the access token is read onto the request after any refresh has completed. The block itself is unchanged — same condition, same error handling, same token-set update — it only runs earlier, and no public API changed.

Verifying it

test/token_refresh.test.ts stubs global.fetch and asserts which Authorization header actually reaches the wire, so it needs no credentials and makes no network calls. It covers both branches of request() (the string-URL path via my.profile() and the RequestConfig body path via submissions.update()), plus two controls that must not refresh: a person token set that has not expired, and an integration token set.

On main at 9127e9f, two of the four fail:

$ npx jest test/token_refresh.test.ts
  ● Person token refresh › uses the refreshed access token on the request that triggered the refresh

    - Expected  - 1
    + Received  + 1

      Array [
    -   "Bearer refreshed-access-token",
    +   "Bearer expired-access-token",
      ]

Tests:       2 failed, 2 passed, 4 total

On this branch all four pass. yarn build and yarn test were run on main first and then here for comparison: build:core succeeds in both, build:types reports the same 50 tsc diagnostics in both and every one of them is inside node_modules/@types/node (none in src), and test/main.test.ts and test/vanilla.test.js fail identically in both because they need CLIENT_SECRET / INTEGRATION_ACCESS_TOKEN / REFRESH_TOKEN. Nothing that was passing changed, and the new suite is the only one that goes from red to green.

One merge note: #38 also edits src/types/common.ts, but for the request-options filter types — it does not touch request() or the files here, so the two should not conflict.

How this was managed

This work was tracked as a story on a board imported from this repository's own issues and pull requests (37 stories, 5 labels): Person token refresh does not apply to the request that triggered it, on the board at eastagiletracker.com/projects/334.

If you'd rather not receive contributions like this, reply no-more-prs on this pull request and we won't open any further ones on your repositories.

---

Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com