Skip to content

[AZINTS-4086] Report token expiry as connection error#59

Merged
benjjs merged 3 commits into
mainfrom
benjamin.johnsonstaub/azints-4077/shortcut-multi-lfo
Oct 28, 2025
Merged

[AZINTS-4086] Report token expiry as connection error#59
benjjs merged 3 commits into
mainfrom
benjamin.johnsonstaub/azints-4077/shortcut-multi-lfo

Conversation

@benjjs
Copy link
Copy Markdown
Collaborator

@benjjs benjjs commented Oct 28, 2025

Motivation

Altan ran into an issue where if you start the flow and then come back after a while (but before our normal time-out, the azure CLI token can expire, even in cloud shell. This leads to an error being thrown and not being handled nicely. We should handle this nicely because it's a situation outside of our control.

Summary

Regardless of what step we're in, check any errors to see if they are a CLI token expiration error. If they are, report it as a connection error instead so that the UI can suggest a refresh instead of indicating that something has failed unexpectedly and that the user should contact support.

Testing

Still working on getting a test for this, it takes a while.

@benjjs benjjs requested review from agulen and gpalmz October 28, 2025 14:11
@benjjs benjjs requested a review from a team as a code owner October 28, 2025 14:11
Copy link
Copy Markdown
Collaborator

@gpalmz gpalmz left a comment

Choose a reason for hiding this comment

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

Couple nits but LGTM

step_complete.set()
if loading_message_thread:
loading_message_thread.join()
if isinstance(e, RuntimeError) and self.EXPIRED_TOKEN_ERROR in repr(e):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not loving the cloud shell specific error handling in the general status reporter but definitely not something to block on

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I don't think this is cloud shell specific? The language just refers to the Azure CLI which is used in either case.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Got it, thanks. I do think it would be nice to share a status reporter across clouds eventually since it'll be hitting the same collection of endpoints once we migrate. I'd envision the status reporter ideally as just a client for that API. We can figure this out later though

@benjjs benjjs merged commit 77c2a5c into main Oct 28, 2025
2 checks passed
@benjjs benjjs deleted the benjamin.johnsonstaub/azints-4077/shortcut-multi-lfo branch October 28, 2025 15:54
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.

3 participants