sachaos / todoist

Todoist CLI Client. I ❤️ Todoist and CLI.
MIT License
1.46k stars 105 forks source link

Unable to perform initial sync due to reminders.is_deleted of wrong type #234

Open opennomad opened 1 year ago

opennomad commented 1 year ago

Using todoist-git in AUR.

$ todoist-cli -v
todoist version 0.20.0

I get the following error:

Error: json: cannot unmarshal number into Go struct field .reminders.is_deleted of type bool

Almost smells like an API issue on the Todoist side returning a number instead of a bool?

HacDan commented 1 year ago

Hmm, I'm not able to reproduce this, but that doesn't mean it isn't happening. I don't use reminders, but I do see that the API documentation says it'll send back a 0 or 1 depending on state.

I created a reminder on a task -> synced -> deleted said reminder -> and finally synced. No errors, but I may even be using reminders wrong, haha!

Is this still happening, @opennomad? If so, I'll go ahead and modify things a bit to see if that solves things for you.

Edit:

I do know that @kenliu was reworking the library to better match the Todoist API as lots of things have changed. Maybe they can chime in here where they're at with that.

kenliu commented 1 year ago

I was looking into #231 which turned out to be a bug in the Todoist API where the incorrect type was being returned (integer instead of string). I'd have to look more closely at this to determine if the same thing is happening here.

kenliu commented 1 year ago

I strongly suspect that whatever was the problem on the server side for Todoist also caused this issue and is now resolved on their end. @opennomad can you try again and see if the problem is still happening?

kenliu commented 1 year ago

So this is interesting...looking at the json returned from the sync API call, the API returns:

"is_deleted": false

screenshot of a part of the json response: image

However, the API documentation clearly states that is_deleted should return 0 or 1 instead of true or false.

So this is kind of a funky situation. The todoist code is expecting the API to return true or false but the API docs specify that 0 or 1 should be returned. Given that the todoist code has been this way for a long time, I think it's likely that the Todoist API has actually been returning incorrect (with respect to the API docs) values for some time. However this doesn't explain what @opennomad reported to open this issue. Would appreciate any other thoughts on this.

HacDan commented 1 year ago

I'd say in this case we either need to write a custom unmarshaler for this field, or we just stop marshalling out that field until the API is fixed.

That item is not used anywhere in the codebase. Just a thought.

opennomad commented 1 year ago

I just tried again, and it still complains about the unmarshal number. This is a new laptop and initial sync.

kenliu commented 1 year ago

That is weird, thanks for the update @opennomad. I like your idea @HacDan -- if we aren't using this field right now then let's remove it to prevent syncing issues until we can figure out what's going on with the API. I can open up a support ticket with Todoist to find out what the API should be returning.

HacDan commented 1 year ago

I put a ticket in with the Todoist Support Team yesterday. I haven't heard anything, though.

Regarding commenting out, I don't think it should have any negative effect on the app.

A quick ripgrep through the code base returned no hits outside of the lib folder for IsDeleted or is_deleted

@opennomad Can you test this build? I understand if you don't want to. I've also uploaded the minor change to GitHub if you'd like to check the code first and/or build it yourself. Nothing funky, just trying to be transparent. I only uploaded an elf 64 bit build. I can upload other builds if need be.

Unfortunately, because I'm not experiencing the issue, I don't know if this will resolve the issue. I only commented out the IsDeleted field for Reminders.

opennomad commented 1 year ago

The initial sync succeeeds:

./todoist-linux-amd64 sync

but then I get a sigsev when I try to list. That happens with both the custom built and the version I had installed already.

./todoist-linux-amd64 l
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0x6db2ab]

goroutine 1 [running]:
github.com/sachaos/todoist/lib.Item.LabelsString({{{{0xc000245632, 0xa}}, {{0xc000245640, 0xa}}, {0xc0001e0280, 0x4a}, {0xc000245650, 0x7}}, {0x0}, {0x0}, ...}, ...)
        /Users/hacdan/Code/todoist/lib/item.go:221 +0x20b
main.List.func1(0xc00029f248, 0x0?)
        /Users/hacdan/Code/todoist/list.go:72 +0x31b
main.traverseItems(0xc00029f248, 0xc000198ff0, 0x0)
        /Users/hacdan/Code/todoist/list.go:13 +0x2e
main.traverseItems(0xc00029f110, 0xc000198ff0, 0x0)
        /Users/hacdan/Code/todoist/list.go:20 +0x74
main.traverseItems(0xc00029efd8, 0xc000198ff0, 0x0)
        /Users/hacdan/Code/todoist/list.go:20 +0x74
main.traverseItems(0xc00029eea0, 0xc000198ff0, 0x0)
        /Users/hacdan/Code/todoist/list.go:20 +0x74
main.traverseItems(0xc00029ed68, 0xc000198ff0, 0x0)
        /Users/hacdan/Code/todoist/list.go:20 +0x74
main.traverseItems(0xc00029ec30, 0xc000198ff0, 0x0)
        /Users/hacdan/Code/todoist/list.go:20 +0x74
main.traverseItems(0xc00029eaf8, 0xc000198ff0, 0x0)
        /Users/hacdan/Code/todoist/list.go:20 +0x74
main.traverseItems(0xc00029e9c0, 0xc000198ff0, 0x0)
        /Users/hacdan/Code/todoist/list.go:20 +0x74
main.traverseItems(0xc00029e888, 0xc000198ff0, 0x0)
        /Users/hacdan/Code/todoist/list.go:20 +0x74
main.traverseItems(0xc00029e750, 0xc000198ff0, 0x0)
        /Users/hacdan/Code/todoist/list.go:20 +0x74
main.traverseItems(0xc00029e618, 0xc000198ff0, 0x0)
        /Users/hacdan/Code/todoist/list.go:20 +0x74
main.traverseItems(0xc00029e4e0, 0xc000198ff0, 0x0)
        /Users/hacdan/Code/todoist/list.go:20 +0x74
main.traverseItems(0xc00029e3a8, 0xc000198ff0, 0x0)
        /Users/hacdan/Code/todoist/list.go:20 +0x74
main.traverseItems(0xc00029e270, 0xc000198ff0, 0x0)
        /Users/hacdan/Code/todoist/list.go:20 +0x74
main.traverseItems(0xc00029e138, 0xc000198ff0, 0x0)
        /Users/hacdan/Code/todoist/list.go:20 +0x74
main.traverseItems(0xc00016b680, 0xc000198ff0, 0x0)
        /Users/hacdan/Code/todoist/list.go:20 +0x74
main.List(0xc000137500)
        /Users/hacdan/Code/todoist/list.go:58 +0x3f6
github.com/urfave/cli/v2.(*Command).Run(0xc0001f0000, 0xc000137500, {0xc0002ca300, 0x1, 0x1})
        /Users/hacdan/go/pkg/mod/github.com/urfave/cli/v2@v2.25.1/command.go:274 +0x9eb
github.com/urfave/cli/v2.(*Command).Run(0xc0001f1340, 0xc000136880, {0xc000138000, 0x2, 0x2})
        /Users/hacdan/go/pkg/mod/github.com/urfave/cli/v2@v2.25.1/command.go:267 +0xc4d
github.com/urfave/cli/v2.(*App).RunContext(0xc0001ee000, {0x960518?, 0xc000126000}, {0xc000138000, 0x2, 0x2})
        /Users/hacdan/go/pkg/mod/github.com/urfave/cli/v2@v2.25.1/app.go:332 +0x616
github.com/urfave/cli/v2.(*App).Run(...)
        /Users/hacdan/go/pkg/mod/github.com/urfave/cli/v2@v2.25.1/app.go:309
main.main()
        /Users/hacdan/Code/todoist/main.go:352 +0x2036
HacDan commented 1 year ago

Hmm...

I'm wondering if this is the other issue that has been cropping up with the - character in label names. If so, this is the same issue as #232

That's where things are going awry anyway.

@opennomad Do you happen to have any - in your label names by chance? Just trying to narrow down one issue from another.

opennomad commented 1 year ago

@HacDan , I might be even more "evil" than a -. I've some repeating daily tasks with a glyph in them:

🌄 Good Morning 🏞️ Daily Groove 🌃 Good Night

I just removed them as a test, and I'm still getting the SIGSEGV.

I then searched for -, and it does show a bunch of tasks with that character in it. Various web sites and a couple of emails I added via the gmail plugin.

FlohGro-dev commented 1 year ago

I get the same / a similar error when I try to add something to todoist:

/usr/local/bin/todoist add "[$1]($2) in DEVONthink"
Error: json: cannot unmarshal bool into Go struct field .reminders.is_deleted of type int
HacDan commented 1 year ago

@opennomad Have you tried blowing away your sync cache, resyncing, and the listing, to see if it makes a difference?

Sync cache is located in ~/.cache/todoist/cache.json

I tried setting up with your labels, but no change here. I'm not entirely convinced it's a special character causing the issue.

@FlohGro-dev I can put a build together for you if you'd like, for testing, to see if that resolves things for you. I'm hesitant to push the PR as I haven't tested it extensively. I would just need to know your os and architecture. I have an elf 64 bit binary linked above if that's what you're running.

krisgry commented 1 year ago

I seem to have the same issue. Tried deleting the sync cache and the API token, but still get the "cannot unmarshal...." error. Hope that me confirming this is of some use.

I have several labels with e.g. emojis.

opennomad commented 1 year ago

I tried blowing the cache away and syncing again. using the default version that comes with arch, it get the unmarshal error on sync. The custom build provided successfully syncs, but attempting to list tasks then fails with the same segvfault as earlier.

HacDan commented 1 year ago

So I finally heard back from Todoist. They have deprecated the v8 sync API and in the v9 sync api, this now returns a boolean instead of an int.

Because of this, the backend library will need to be reworked a bit to accommodate the new API. I'll begin work on this, but if someone beats me to it, feel free!

kenliu commented 1 year ago

thanks @HacDan! This might be a good opportunity to fix #222 which I did a little bit of work on and then didn't come back to. I won't be bothered if you fix it first!

opennomad commented 7 months ago

I just noticed that the AUR version if have installed (r381.15ce568-1) can sync and list etc. It seems fully functional and resolves my original issue reported.