Skip to content

Make duck player page more resilient to race conditions on macOS #571

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

Merged
merged 1 commit into from
Jun 14, 2023

Conversation

shakyShane
Copy link
Contributor

@shakyShane shakyShane commented Jun 13, 2023

When implementing Duck Player on macOS - I encountered a race-condition where our script in the page is calling for a request/response before the special page has been marked as 'safe to receive messages' by the native side.

I looked into fixing it on their end, but it's heavily related to the fact that user-scripts are global and cannot be directly tied to a particular hostname.

So, I'm fixing it here by attempting (10 times) to make an 'initial connection' - once that's established we continue as normal.

This is a nice way of ensuring we can proceed across all platforms. So whilst it's technically a macOS only fix right now, it really helps everywhere by not giving up on first error.

@shakyShane
Copy link
Contributor Author

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@shakyShane shakyShane force-pushed the shane/duckplayer-fixes branch from 1afad5a to cc176b0 Compare June 13, 2023 21:28
@shakyShane shakyShane changed the title make duck player page more resistant to race conditions on macOS Make duck player page more resilient to race conditions on macOS Jun 13, 2023
Copy link
Contributor

@jonathanKingston jonathanKingston left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@shakyShane shakyShane merged commit e334bea into main Jun 14, 2023
@shakyShane shakyShane deleted the shane/duckplayer-fixes branch June 14, 2023 15:17
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