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

refactor: [M3-6852] - Remove global error interceptors #10850

Conversation

hkhalil-akamai
Copy link
Contributor

@hkhalil-akamai hkhalil-akamai commented Aug 28, 2024

Description 📝

Today the Cloud Manager uses a global error interceptor to add links to APIErrors that call for users to open a support ticket. This interceptor introduces type unsafety (since it necessitates a cast) and reduces flexibility in displaying error messages.

#10684 made progress towards reducing our reliance on this interceptor by using the <ErrorMessage /> component, which formats errors at the component level. This PR completes that refactor by removing the two remaining cases where errors were being intercepted:

  • Errors about the user reaching their thing limit
    • These are now handled at the component level for all entities (Linodes, firewalls, volumes, etc.)
  • Errors caused when trying to migrate in an account with the dc_migrations_disabled customer tag
    • This is now handled in the MigrateLinode component

Changes 🔄

  • Deletes the global error interceptor in request.tsx and associated utilities (interceptAPIError.ts)
  • Detect migration errors in the ErrorMessage component
  • Add ErrorMessage in a couple of missing places (GenerateFirewallDialog, MigrateLinode)

Target release date 🗓️

9/17

How to test 🧪

In all of the following instances, displayed errors should include a link to the SupportTicketDialog. For example:

image

  • Add the dc_migrations_disabled tag to your account and attempt to migrate a Linode
  • Reduce your thing limit/reputation in admin and attempt to create new services (e.g., Linodes, volumes, firewalls, etc.)
  • Verify APIErrors otherwise appear as expected across the app

@hkhalil-akamai hkhalil-akamai self-assigned this Aug 28, 2024
@hkhalil-akamai hkhalil-akamai requested a review from a team as a code owner August 28, 2024 21:13
@hkhalil-akamai hkhalil-akamai requested review from cpathipa and jaalah-akamai and removed request for a team August 28, 2024 21:13
Copy link

github-actions bot commented Aug 28, 2024

Coverage Report:
Base Coverage: 86.15%
Current Coverage: 86.21%

Copy link
Member

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

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

Nice clean up! 🧼

Copy link
Contributor

@mjac0bs mjac0bs left a comment

Choose a reason for hiding this comment

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

Thanks for doing this, @hkhalil-akamai! The migrate and support errors are looking good and opening the support ticket dialog at the component level.

There's one minor bit of clean up from a change I made in a previous PR due to the casting - we could revert it now with better type safety.

@mjac0bs mjac0bs added Approved Multiple approvals and ready to merge! and removed Ready for Review labels Aug 30, 2024
@hkhalil-akamai hkhalil-akamai merged commit a8f7cd9 into linode:develop Sep 3, 2024
19 checks passed
@hkhalil-akamai hkhalil-akamai deleted the M3-6852-remove-global-error-interceptors branch September 3, 2024 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants