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

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

Open
opennomad opened this issue May 5, 2023 · 18 comments
Open
Labels

Comments

@opennomad
Copy link

opennomad commented May 5, 2023

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
Copy link
Contributor

HacDan commented May 7, 2023

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
Copy link
Collaborator

kenliu commented May 7, 2023

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 kenliu changed the title Unable to perform initial sync due to reminiders.is_deleted of wrong type Unable to perform initial sync due to reminders.is_deleted of wrong type May 8, 2023
@kenliu
Copy link
Collaborator

kenliu commented May 8, 2023

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 kenliu added the bug label May 8, 2023
@kenliu
Copy link
Collaborator

kenliu commented May 9, 2023

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
Copy link
Contributor

HacDan commented May 9, 2023

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
Copy link
Author

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

@kenliu
Copy link
Collaborator

kenliu commented May 10, 2023

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 added a commit to HacDan/todoist that referenced this issue May 10, 2023
… api is being funky

Commented out IsDeleted field in the Reminder struct as the API is currently being inconsistent. Work around for issue sachaos#234.
@HacDan
Copy link
Contributor

HacDan commented May 10, 2023

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
Copy link
Author

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
Copy link
Contributor

HacDan commented May 12, 2023

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
Copy link
Author

@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
Copy link

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
Copy link
Contributor

HacDan commented May 17, 2023

@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
Copy link

krisgry commented May 18, 2023

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
Copy link
Author

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
Copy link
Contributor

HacDan commented Jun 2, 2023

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
Copy link
Collaborator

kenliu commented Jun 2, 2023

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants