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

Wait for Chelonia, more error logs #2471

Conversation

corrideat
Copy link
Member

Closes #2469.

This PR adds more logging to catch Chelonia set up errors, and it adds logic to check that Chelonia has been setup.

@corrideat corrideat force-pushed the 2469-missing-pubsub-instance-bug-and-failure-to-recover-pubsub-connection branch from 78f539f to 4250b55 Compare December 18, 2024 16:05
Copy link

cypress bot commented Dec 18, 2024

group-income    Run #3594

Run Properties:  status check passed Passed #3594  •  git commit 344a622c51 ℹ️: Merge bdd958fe176443f2129ef051ceef0e100c755357 into 8aad62eadf34318af92b8363d142...
Project group-income
Branch Review 2469-missing-pubsub-instance-bug-and-failure-to-recover-pubsub-connection
Run status status check passed Passed #3594
Run duration 11m 40s
Commit git commit 344a622c51 ℹ️: Merge bdd958fe176443f2129ef051ceef0e100c755357 into 8aad62eadf34318af92b8363d142...
Committer Ricardo Iván Vieitez Parra
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 10
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 111
View all changes introduced in this branch ↗︎

@corrideat corrideat requested a review from taoeffect December 18, 2024 16:22
@corrideat corrideat force-pushed the 2469-missing-pubsub-instance-bug-and-failure-to-recover-pubsub-connection branch from 4250b55 to b4d04e3 Compare December 18, 2024 16:23
Comment on lines +54 to +81
// Send a 'ready' message to the SW and wait back for a response
// This way we ensure that Chelonia has been set up
await new Promise((resolve, reject) => {
const messageChannel = new MessageChannel()
messageChannel.port1.onmessage = (event) => {
if (event.data.type === 'ready') {
resolve()
} else {
reject(event.data.error)
}
messageChannel.port1.close()
}
messageChannel.port1.onmessageerror = () => {
reject(new Error('Message error'))
messageChannel.port1.close()
}

navigator.serviceWorker.ready.then((worker) => {
worker.active.postMessage({
type: 'ready',
port: messageChannel.port2
}, [messageChannel.port2])
}).catch((e) => {
reject(e)
messageChannel.port1.close()
})
})

Copy link
Member

@taoeffect taoeffect Dec 18, 2024

Choose a reason for hiding this comment

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

This seems like boilerplate we could abstract away in a function somewhere.

Can you create a function so that chunk is shortened like so?

await postWorkerMessage(worker, { type: 'ready' })

And if there are any other parts of the app that also do something like this, they can reuse this function.

Copy link
Member Author

Choose a reason for hiding this comment

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

I could make it into another function, but this particular message is unique in how it's sent, that it expects a response and that it expects a response (sbp also expects a response, but is handled by a more general-purpose function)

Copy link
Member Author

Choose a reason for hiding this comment

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

It's also not entirely general purpose because the handler for onmessage is custom

@corrideat corrideat marked this pull request as ready for review December 18, 2024 19:04
Copy link
Member

@taoeffect taoeffect left a comment

Choose a reason for hiding this comment

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

Great work @corrideat !

@taoeffect taoeffect merged commit 6b1ad58 into master Dec 18, 2024
4 checks passed
@taoeffect taoeffect deleted the 2469-missing-pubsub-instance-bug-and-failure-to-recover-pubsub-connection branch December 18, 2024 19:20
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.

"Missing pubsub instance" bug and failure to recover pubsub connection
2 participants