alanvardy / tod

An unofficial Todoist command line client written in Rust
MIT License
104 stars 9 forks source link

Bug: Tod crashes with parsing task durations - only handles "day" #866

Closed stacksjb closed 1 month ago

stacksjb commented 1 month ago

The following filter works in Todoist, but does not in Tod:

@Computer & !recurring

I've discovered that any filter using "!" (not) is incorrectly parsed by Tod and breaks the code

Likely the filter prompt needs to be converted to a string literal ({{}}). I attempted escaping just the ! with ! but that did not work.

EDIT: see latest comment - the issue does not have to do with filter, but has to do with task durations.

stacksjb commented 1 month ago

Verbose testing and manually testing the request shows that it does appear that the filter is being encoded successfully, as the correct request is being made, as the following requests are both correct and work when attempted manually

https://api.todoist.com/rest/v2/tasks?filter=%21recurring or even https://api.todoist.com/rest/v2/tasks?filter=!recurring' works as well (it appears we are URL encoding all characters, not just spaces and reserved ones)

In addition if I run in verbose/debug, it appears the results are returned successfully, before Tod crashes with the error invalid value: integer8699, expected u8 at line 68945 column 17

`➜ Documents tod list view

Select Project or Filter: Filter Enter a filter: !recurring

Error from serde_json: invalid value: integer 8699, expected u8 at line 68945 column 17 ➜ Documents`

The exact error location doesn't always appear consistently - running with another one of my filters that also contains the "!recurring" gives this error:

invalid value: integer9240, expected u8 at line 922 column 17

stacksjb commented 1 month ago

This appears to be a regression bug as this error does not occur on earlier versions... tested on 0.5.10 and does not occur. Testing backwards to identify which release this was introduced in.

stacksjb commented 1 month ago

This appears to have been introduced in version 0.5.11 (https://github.com/alanvardy/tod/tree/8a18d5769a471be641f7bdaf7b193d40fabbead2) - it works in 0.5.10 but not in 0.5.11.

stacksjb commented 1 month ago

Issue was introduced in commit https://github.com/alanvardy/tod/commit/e471bd6a3360e47f49888380e7fca2848cf253fb "Print Task Durations" - prior commit works.

stacksjb commented 1 month ago

@alanvardy are you able to look into this commit? Thx!

stacksjb commented 1 month ago

After looking into this and the error, this bug was not related to the filter in use, but rather the task being returned having a specific duration. I was able to reproduce it with tasks with the same duration - I think Tod is not handling tasks which are returning a task with the "minute" duration. Here's the durations I had on the tasks (which matched up to the error being printed.)

stacksjb commented 1 month ago

Todoist supports the following task durations (see https://todoist.com/help/articles/set-a-task-duration-L1kYkZv8d) Minute (m) Hour (h) Day (d)

My tasks contained durations including:

8699|minute
10079|minute
6|day
9240|minute
8699|minute

The code only handles Day.

alanvardy commented 1 month ago

Thank you for the bug report! I'll work on getting a fix up :+1:

alanvardy commented 1 month ago

Issue was that I used a u8 to represent the amount when the number could be far higher. Changed to a u32!

Thanks again, released 0.6.13 to resolve the issue.