NewtonFernandis commented on PR #35143:
URL: https://github.com/apache/superset/pull/35143#issuecomment-4778019110
> Thanks @NewtonFernandis — the cleanup itself looks right. One snag: in the
`catch` you reschedule with `getGuestTokenRefreshTiming(newGuestToken)`, but
`newGuestToken` is scoped to the `try`, so it's undefined there. Mind fixing
that, running pre-commit, and rebasing? Thanks!
Before pushing a fix, I went to rebase this branch onto `master` and hit a
conflict that turned out to be informative: **master has already landed a more
complete version of this work.** Its `refreshGuestToken` tracks the pending
timer and cancels it on unmount (the cleanup this branch was for), and its
`catch` schedules the retry with a `DEFAULT_TOKEN_REFRESH_RETRY_MS` constant
rather than a token-derived delay, so the out-of-scope `newGuestToken` reference
doesn't exist there at all. The bug is already gone upstream.
Master also went a bit further:
- a `withTimeout` wrapper around `fetchGuestToken`, and
- iframe teardown if the initial fetch rejects after mount.
So this branch looks superseded — I couldn't find anything here that
master is missing. I'd suggest we close this in favor of what's on `master`.
Happy to do a focused diff of the two implementations first if you want to
confirm
nothing's lost.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
