8000 Prevented foobar crash when exiting during AllMusic fetching by TT-ReBORN · Pull Request #8 · Wil-B/Biography · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Prevented foobar crash when exiting during AllMusic fetching #8

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 te 8000 rms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

TT-ReBORN
Copy link

As title says, more info here:
https://hydrogenaud.io/index.php/topic,121047.msg1044579.html#msg1044579

@regorxxx, you can close your open PR as it is with this PR deprecated: #7

Co-authored-by: regorxxx <regorxxx@users.noreply.github.com>
@regorxxx
Copy link
Contributor
regorxxx commented Jun 5, 2024

Closed it.

@TT-ReBORN just to confirm, the current code handles only a single request at a time. Is that intended?
Just asking, as far as I see the previous one allowed to send multiple request when changing selection and processing them on the background via promises (correct me if I'm wrong, but at least that seems the flow).
Your PR is not only a fix but a change on behavior, since every send call aborts the previous request instead of pushing them into a stack and aborting the entire stack on the unload callback (which was my proposed solution).

@TT-ReBORN
Copy link
Author

Yes, it should process single requests and start fresh again.
Imo this is the cleanest way and performance wise ( client <-> server side ), we do not need to have multiple request
processed because we only fetch text ( which is instant ) and no images like last.fm does.
I do not see any side effects, I have tested it with Majestyk and we didn't encounter any issues.

regorxxx added a commit to regorxxx/Biography that referenced this pull request Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0