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

Replace invalid timezone with sensible default #2718

Closed
szokeasaurusrex opened this issue Nov 13, 2023 · 5 comments
Closed

Replace invalid timezone with sensible default #2718

szokeasaurusrex opened this issue Nov 13, 2023 · 5 comments

Comments

@szokeasaurusrex
Copy link
Member

While working on getsentry/sentry-cli#1807, I noticed that when an invalid timezone string is provided when configuring a cron monitor via monitor upserts, the monitor silently fails to be created in Sentry. This could lead to a scenario where a user, who has a typo in their timezone string, thinks that they have created a cron monitor. But, when they later go to Sentry to check on it, they will notice that their cron monitor is not actually there, and they will have no idea why.

So, we should check in Relay whether the timezone string corresponds to a valid timezone, and if it does not, we should replace it with a sensible default (such as UTC time). Probably, the best option would be to treat an invalid timezone input as an absent timezone input, since we already are correctly handling the case of no timezone being provided.

@evanpurkhiser
Copy link
Member

This may make more sense to be handled in our consumer

@jan-auer
Copy link
Member

Can you share a repro, please?

@olksdr
Copy link
Contributor

olksdr commented Nov 27, 2023

Please, re-open if there is a repro or any additional information we can act on. Thank you.

@olksdr olksdr closed this as completed Nov 27, 2023
@Dav1dde
Copy link
Member

Dav1dde commented Nov 28, 2023

We looked into it, Relay does not touch or parse the timezone. Additional validation will be added to the cli.

@szokeasaurusrex
Copy link
Member Author

szokeasaurusrex commented Nov 28, 2023

I have opened getsentry/sentry-cli#1845 to validate the timezone string directly within the CLI.

@szokeasaurusrex szokeasaurusrex closed this as not planned Won't fix, can't repro, duplicate, stale Nov 28, 2023
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

No branches or pull requests

5 participants