fix(sources): fallback resolve when callback is never called#2407
fix(sources): fallback resolve when callback is never called#2407
Conversation
Some community sources override `source:resolve` but may never call the
callback, e.g. they check an option that ends up being nil and causes
the async task to hang indefinitely (since it's waiting on a callback
that will never come):
```lua
function source:resolve(item, callback)
if self.opts.resolve then -- this may be nil
self.opts.resolve(item, callback) -- callback never called
end
end
```
The fix detects this by checking if the callback was invoked
synchronously after resolve returns. If not, and there’s no return value
indicating async work, we fall back to resolving with the original item.
I believe returning the original item is a safer option, as it avoids
indefinite hangs without requiring to strictly follow the resolve
contract.
|
I'd like to avoid patching bugs in sources in blink.cmp itself. I think the issue might be that there's no feedback that the plugin has a bug in it. Perhaps we could have a timeout of 30s and throw an error if the callback isn't called? |
|
Good point about the timeout! I added it alongside the original sync check for full coverage, but kept fallback + warning instead of throwing an error (feels safer for UX). 30s for timeout seem long I set it to 3s instead but feel free to adapt if I miss a case. The main goal of this PR is to avoid deadlocks. If the callback never fires, the resolve task hangs forever and causes weird issues like misplaced documentation window (detected with faulty source while fixing window artifacts in the other PR). The warnings explicitly name the buggy source so users don't file Thoughts? |
2 participants
Some community sources override
source:resolvebut may never call the callback, e.g. they check an option that ends up being nil and causes the async task to hang indefinitely (since it's waiting on a callback that will never come):The fix detects this by checking if the callback was invoked synchronously after resolve returns. If not, and there’s no return value indicating async work, we fall back to resolving with the original item.
I believe returning the original item is a safer option, as it avoids indefinite hangs without requiring to strictly follow the resolve contract.