Skip to content

fix(node/browser): Drop session if release is not a string #3396

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Apr 16, 2021
Merged

fix(node/browser): Drop session if release is not a string #3396

merged 2 commits into from
Apr 16, 2021

Conversation

ahmedetefy
Copy link
Contributor

@ahmedetefy ahmedetefy commented Apr 15, 2021 •
edited
Loading

This PR fixes:

  • Sending release as an object to the Server when release is set on SDK initialisation as an object.

Expected Behaviour:

  • Release should be sent to the server as a string

Ref:
https://sentry.my.sentry.io/organizations/sentry/issues/286540/events/769bff308563499d8f807b67e787faf2/?project=4

@ahmedetefy ahmedetefy marked this pull request as ready for review April 15, 2021 13:07
@ahmedetefy ahmedetefy requested a review from kamilogorek as a code owner April 15, 2021 13:07
@github-actions
Copy link
Contributor

github-actions bot commented Apr 15, 2021 •
edited
Loading

size-limit report

Path Size
@sentry/browser - CDN Bundle (gzipped) 20.6 KB (+0.06% 🔺)
@sentry/browser - Webpack 21.46 KB (+0.07% 🔺)
@sentry/react - Webpack 21.5 KB (+0.07% 🔺)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped) 27.81 KB (+0.05% 🔺)

@ahmedetefy ahmedetefy closed this Apr 15, 2021
@ahmedetefy ahmedetefy reopened this Apr 16, 2021
@ahmedetefy ahmedetefy requested review from a team and rhcarvalho and removed request for a team April 16, 2021 08:35
kamilogorek
kamilogorek suggested changes Apr 16, 2021
View reviewed changes
Copy link
Contributor

@kamilogorek kamilogorek left a comment

Choose a reason for hiding this comment

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

Per slack convo

@ahmedetefy ahmedetefy closed this Apr 16, 2021
@ahmedetefy ahmedetefy reopened this Apr 16, 2021
@ahmedetefy ahmedetefy requested a review from kamilogorek April 16, 2021 11:07
@ahmedetefy ahmedetefy merged commit 8b4a7f2 into master Apr 16, 2021
@ahmedetefy ahmedetefy deleted the fix-release-object branch April 16, 2021 12:01
@rhcarvalho rhcarvalho changed the title fix(node/browser): Stringify options release if release is passed as an object rather than a string fix(node/browser): Drop session if release is not a string Apr 16, 2021
@ahmedetefy
Copy link
Contributor Author

ahmedetefy commented Apr 16, 2021 •
edited
Loading

For sessions, We have decided to drop them (not send them), and log that they were dropped because release was either missing or non string because (server will drop them anyways)*, and they won't be available in the UI to attach a message to them

And I asterisk this because relay has the concept of what is called lenientString which would also accept floats and convert them to string for example

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rhcarvalho rhcarvalho rhcarvalho left review comments

@kamilogorek kamilogorek kamilogorek approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ahmedetefy @rhcarvalho @kamilogorek