Skip to content

Check the output of _scheduled_refresh to prevent TypeError#8518

Open
TheoMathurin wants to merge 2 commits intoholoviz:mainfrom
TheoMathurin:prevent-expiry-typerror
Open

Check the output of _scheduled_refresh to prevent TypeError#8518
TheoMathurin wants to merge 2 commits intoholoviz:mainfrom
TheoMathurin:prevent-expiry-typerror

Conversation

@TheoMathurin
Copy link
Copy Markdown
Contributor

@TheoMathurin TheoMathurin commented Mar 16, 2026

Description

Fixes #8517

Several ways to go here, I went for the replacement of the user not in state._oauth_user_overrides condition for a direct check on the output of _scheduled_refresh. The former is only one path that leads to the outputs being None.

Also I chose to check on every single of the three outputs to be sure, but that's probably not strictly necessary.

Happy to consider alternatives. We could for instance, when the refresh fails, choose to raise an exception and catch it.

How Has This Been Tested?

As I could not create an MRE, I could not test but the logic is pretty straightforward.

@TheoMathurin TheoMathurin changed the title Check the output of _scheduled_refresh to prevent TypeError Check the output of _scheduled_refresh to prevent TypeError Mar 16, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 16, 2026

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.13%. Comparing base (e6c2916) to head (2f78412).

Files with missing lines Patch % Lines
panel/auth.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8518      +/-   ##
==========================================
- Coverage   86.24%   86.13%   -0.11%     
==========================================
  Files         349      349              
  Lines       55388    55388              
==========================================
- Hits        47769    47711      -58     
- Misses       7619     7677      +58     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@philippjfr
Copy link
Copy Markdown
Member

I think there could be OAuth implementations that do not provide a refresh_token, which means this new condition would always return None and cause an auth loop. I think the same could be true of the expiry.

@TheoMathurin
Copy link
Copy Markdown
Contributor Author

TheoMathurin commented Mar 17, 2026

Indeed, what about just checking on access_token then? We could also make the function return a single None instead of three and rearrange the bits where it's called.

@philippjfr
Copy link
Copy Markdown
Member

That makes sense to me.

@TheoMathurin
Copy link
Copy Markdown
Contributor Author

Can we rerun the tests? I don't think the failures are related to these changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TypeError sometimes raised after scheduled OAuth refresh

2 participants