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

feat(expo) better script state for upload #3568

Merged
merged 3 commits into from
Jan 30, 2024

Conversation

quinlanj
Copy link
Contributor

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

I was looking through the repo issues and noticed someone had problems with uploading their sourcemaps to Sentry #3546. The expo upload script reported everything was fine even though no sourcemaps were uploaded.

This PR:

  • Prints a green checkmark when all the sourcemaps and assets are present
  • Print warnings when sourcemaps are missing.
  • When all sourcemaps are missing, prompts to tell user to add a --dump-sourcemap flag.

💡 Motivation and Context

💚 How did you test it?

test project

Happy case

⬆️ Uploading dist/_expo/static/js/ios/AppEntry-50bca667cd580a120248b3b22f26def6.hbc bundle and sourcemap...
...
✅ Uploaded bundles and sourcemaps to Sentry successfully.

Partially missing sourcemaps

❓ Sourcemap for dist/_expo/static/js/android/AppEntry-3b9811f2869bc9ddc4264abc7f4e2f8c.hbc not found, skipping...
...
⚠️  Uploaded 1 of 2 bundles and sourcemaps. 

Completely missing sourcemaps

❓ Sourcemap for dist/_expo/static/js/ios/AppEntry-50bca667cd580a120248b3b22f26def6.hbc not found, skipping...
...
⚠️  Uploaded 0 of 2 bundles and sourcemaps. Ensure you are running `expo export` with the `--dump-sourcemap` flag.

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
  • All tests passing
  • No breaking changes

🔮 Next steps

Copy link
Member

@krystofwoldrich krystofwoldrich left a comment

Choose a reason for hiding this comment

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

@quinlanj Thank you, this is a good addition that will minimize the number of issues our users can run into.

@krystofwoldrich krystofwoldrich merged commit 9c786a3 into getsentry:main Jan 30, 2024
22 of 44 checks passed
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