Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve dev-session retry and error report #5226

Merged
merged 1 commit into from
Jan 21, 2025

Conversation

isaacroldan
Copy link
Contributor

@isaacroldan isaacroldan commented Jan 17, 2025

WHY are these changes introduced?

Improves error handling and token refresh mechanisms in the dev session flow.

WHAT is this pull request doing?

  • Introduces proper error handling for authentication failures during dev sessions
  • Adds retry with token refresh ONLY for dev session creation and updates mutations.
  • Improves error messages for expired auth sessions
  • Improves debug output formatting

How to test your changes?

  1. Start a dev session and let it expire (you can intentionally put a bad token in the dev session update mutation to force an authentication error)
  2. Verify that you receive a clean error

Measuring impact

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes

@isaacroldan isaacroldan marked this pull request as ready for review January 17, 2025 13:27
@isaacroldan isaacroldan requested a review from a team as a code owner January 17, 2025 13:27
Copy link
Contributor

We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run "pnpm changeset add" to track your changes and include them in the next release CHANGELOG.

Copy link
Contributor

github-actions bot commented Jan 17, 2025

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
75.25% (-0.01% 🔻)
8889/11812
🟡 Branches
70.43% (-0.02% 🔻)
4332/6151
🟡 Functions
75.1% (+0.02% 🔼)
2335/3109
🟡 Lines
75.81% (-0% 🔻)
8400/11081
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟢
... / app-event-watcher.ts
93.83% (-1.23% 🔻)
86.49% (-2.7% 🔻)
90.48% 98.61%
🟢
... / dev-session.ts
85.61% (-0.68% 🔻)
67.19% (-0.05% 🔻)
88.57% (-0.71% 🔻)
91.23% (-1.22% 🔻)
🟢
... / ConcurrentOutput.tsx
98.36% (-1.64% 🔻)
88% (-4% 🔻)
100%
98.33% (-1.67% 🔻)

Test suite run success

2004 tests passing in 905 suites.

Report generated by 🧪jest coverage report action from 3388086

Copy link
Contributor

@gonzaloriestra gonzaloriestra left a comment

Choose a reason for hiding this comment

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

Not sure if I'm understanding the session handling or maybe something is not working as expected. But if I break the token, I'm getting the session expired error. Shouldn't it start the login flow?

@isaacroldan isaacroldan added this pull request to the merge queue Jan 21, 2025
Merged via the queue into main with commit e27d778 Jan 21, 2025
28 checks passed
@isaacroldan isaacroldan deleted the 01-17-improve_dev-session_retry_and_error_report branch January 21, 2025 12:14
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.

2 participants