nathonius / obsidian-github-link

https://github.com/users/nathonius/projects/8
The Unlicense
29 stars 7 forks source link

Scrolling a document with a few tens of links rapidly exhausts the API limit #69

Closed nikclayton closed 6 months ago

nikclayton commented 7 months ago

From what I can tell from reading the code the plugin doesn't perform any caching of the details returned from the API.

So if you have a page with a few tens of links scattered throughout (say, keeping track of open issues or PRs), and scroll through that document the plugin will make a new GitHub API request every time one of those links scrolls in to the viewport.

This can very rapidly run up against GitHub API rate limits (which relates to https://github.com/nathonius/obsidian-github-link/issues/21), and then the plugin effectively stops working.

nathonius commented 7 months ago

@nikclayton can you give some more details on the symptoms and behavior you're seeing? Does obsidian freeze up? Is this in live preview mode?

API responses are being cached, so something else could be going on. But if the cache expired on all of the results on a page, it could definitely call a bunch at once.

nikclayton commented 7 months ago

Obsidian doesn't freeze, but the "chips" the plugin adds to replace the bare links stop updating.

I had a file with a few tens of links to PRs and issues, and not all of them were converted by the plugin as I scrolled.

E.g., all the links on the first visible part of the document were converted.

So were some that appeared when I scrolled down. Eventually they stopped being converted, just appearing as the bare links, and if I scrolled back up the links that previously had been converted were now showing as bare links.

If I closed the file and reopened it none of the links were converted.

If I then made a request to GitHub from the command line to check rate limits it showed 0 API calls left, with new ones available 20 minutes later.

So this is partly the existing "handle rate limits better" issue that's been opened, but if the API results are supposed to be cached that's not the behaviour I was seeing (otherwise I would expect that previously converted links would continue to display as chips after scrolling back up, or close and reopening the file).

nathonius commented 7 months ago

@nikclayton turns out caching of individual issues and pull requests was not functioning; I've resolved that. Additionally, I've added some safety guards around the rate limit as well as queuing requests instead of running them all concurrently. These changes are available in v0.5.1.

If you're still having trouble with rate limits, what sort of token are you using? Using a token from the github cli should give you a hearty rate limit of 5000 by default, I'd recommend that. That also means it would be your rate limit and you wouldn't be competing with anyone else using the plugin - if using the token generation flow within the plugin, it uses a shared limit.

To see the rate limit, you can enable debug logging in plugin settings. Each request and response will be logged to the console in obsidian. The response headers will include x-ratelimit-limit, x-ratelimit-remaining, x-ratelimit-reset, and x-ratelimit-used. If x-ratelimit-limit has a value of 60, that likely means the token isn't configured correctly.

The oauth app limit hasn't been an issue yet that I'm aware of, but it's definitely a possibility if enough people pick up the plugin.

nikclayton commented 7 months ago

Those changes have helped in 0.5.1, thanks.

I did notice that if the token rate limit expires while the plugin is decorating links in a document there doesn't appear to be a way to get it to restart on the links that were missed, except for closing and re-opening Obisidian (closing and reopening the Markdown file in Obsidian didn't do it).

That caused all the links in the file to be reprocessed, so it doesn't seem as though the cache persists across restarts. On the one hand that meant that it did eventually re-decorate all the links in the file, but on the other hand having to restart Obsidian is pretty heavyweight.

I also couldn't find a way to trigger a refresh on a single URL: E.g., with this in the file:

- [ ] https://github.com/nathonius/obsidian-github-link/issues/69

I can't trigger a "Check this single URL and update the UI if the issue state has changed"

nathonius commented 6 months ago

Closing this in favor of #85 and #86, which should solve the remaining issues described.

nathonius commented 6 months ago

👀 #89

nathonius commented 6 months ago

@nikclayton with the release of v0.6.0, this should be even further improved. Cache is now saved to disk, so upon restart Obsidian won't overload the GitHub API with requests. There can still be some initial issues with rate limit, but by using a simple cache for back-to-back requests and using GitHub's etag and last-modified headers for other requests, data will be kept up to date, but won't count against the rate limit.

Let me know what you think!

nikclayton commented 6 months ago

Just tried it and it's a much better experience. Letting a file of PR links populate with the data, close and re-open Obsidian, and the PR data repopulates much faster.

Thank you.

nathonius commented 3 months ago

@all-contributors please add @nikclayton for bug

allcontributors[bot] commented 3 months ago

@nathonius

I've put up a pull request to add @nikclayton! :tada: