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 date format panic #167

Merged
merged 3 commits into from
Feb 10, 2023
Merged

fix date format panic #167

merged 3 commits into from
Feb 10, 2023

Conversation

Bas-Man
Copy link
Contributor

@Bas-Man Bas-Man commented Feb 9, 2023

  • Remove: unwrap() as this may fail
  • Fix: Because Date parsers can fail, match case

The code will still panic in debug mode. It will close cleanly in production

███████╗██╗███╗   ██╗███████╗
╚══███╔╝██║████╗  ██║██╔════╝
  ███╔╝ ██║██╔██╗ ██║█████╗  
 ███╔╝  ██║██║╚██╗██║██╔══╝  
███████╗██║██║ ╚████║███████╗
╚══════╝╚═╝╚═╝  ╚═══╝╚══════╝
                             

listening on http://127.0.0.1:3000
Error: The date value 2023-1-31 is invalid: the 'month' component could not be parsed for key `article` at line 6 column 1

This should close #164

It is possible for this unwrap to panic.
Replaced with a match to hand the success and fail case.
The Err() may still be improved.
This Err() still needs to be handled more correctly. I think.
@Bas-Man Bas-Man mentioned this pull request Feb 9, 2023
Copy link
Member

@Folyd Folyd left a comment

Choose a reason for hiding this comment

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

Can we also support formats like "2023-1-31" or "2023-1-1", it is better than simply printing errors. What do you think?

@Bas-Man
Copy link
Contributor Author

Bas-Man commented Feb 9, 2023 via email

@Folyd
Copy link
Member

Folyd commented Feb 9, 2023

Yes, fair enough. 👍

@Bas-Man
Copy link
Contributor Author

Bas-Man commented Feb 9, 2023

In addition. If it was ever decided to add time. There would be more issues and breakages.

@Folyd
Copy link
Member

Folyd commented Feb 10, 2023

Don't forget to run cargo fmt, thanks. @Bas-Man

@Bas-Man
Copy link
Contributor Author

Bas-Man commented Feb 10, 2023

Sorry. I will try to get into the habit.
I might add a precommit to run it for me.

Copy link
Member

@Folyd Folyd left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@Folyd Folyd merged commit 0fff4ee into zineland:master Feb 10, 2023
@Bas-Man Bas-Man deleted the fix-date-format-panic branch February 15, 2023 07:55
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.

Fix invalid date panic
2 participants