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

fix: Discard prior transactions on react navigation dispatch #2053

Merged
merged 2 commits into from
Feb 11, 2022

Conversation

jennmueng
Copy link
Member

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

When multiple navigation actions are dispatched before the routeChangeTimeoutMs, and the state does not change occur for a prior action, the transaction is not cancelled and sent with the blank "Route Change" transaction context. This change makes sure any navigation transaction that had a successful navigation state change is removed from the instrumentation (it'll still be present on the scope until finished) and any new navigation action dispatch will cancel any prior waiting transactions to ensure they won't be sent when they don't have a successful state change.

💡 Motivation and Context

Fixes #2051

💚 How did you test it?

Added a jest test where multiple navigation actions were dispatched in a row

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • All tests passing
  • No breaking changes

🔮 Next steps

As we did not recieve a reproduction, need to ask customer to see if bug is fixed.

Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

The e2e for iOS broken is expected due to #2052
But Android should be green but seems to be flaky due to:

Error: The template is not valid. .github/workflows/e2e.yml (Line: 34, Col: 16): hashFiles('**/yarn.lock') couldn't finish within 120 seconds.

@bruno-garcia
Copy link
Member

image
We're good!

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.

Users see transactions name Route Change in Sentry.
2 participants