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

fix(mac): Wrap hditutil detach in retry w/ backoff #7600

Merged
merged 2 commits into from
Jun 11, 2023

Conversation

roryabraham
Copy link
Contributor

We are experiencing a lot of flakiness with our Electron deploys in a GitHub Actions macos-12 runner. This PR is an attempt to wrap the failing command in a retry, as has been done elsewhere in this repo.

I also added a simple backoff mechanism to the retry function.

@changeset-bot
Copy link

changeset-bot bot commented Jun 6, 2023

🦋 Changeset detected

Latest commit: 2e5a91b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 10 packages
Name Type
builder-util Patch
dmg-builder Patch
app-builder-lib Patch
electron-builder-squirrel-windows Patch
electron-builder Patch
electron-publish Patch
electron-forge-maker-appimage Patch
electron-forge-maker-nsis-web Patch
electron-forge-maker-nsis Patch
electron-forge-maker-snap Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@netlify
Copy link

netlify bot commented Jun 6, 2023

Deploy Preview for car-park-attendant-cleat-11576 ready!

Name Link
🔨 Latest commit 2e5a91b
🔍 Latest deploy log https://app.netlify.com/sites/car-park-attendant-cleat-11576/deploys/6486327bbdb29f0008309d77
😎 Deploy Preview https://deploy-preview-7600--car-park-attendant-cleat-11576.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@roryabraham
Copy link
Contributor Author

First-time contributor here, so happy to do whatever is needed to get this relatively simple change ready for review.

@mmaietta mmaietta changed the title Wrap detach in retry w/ backoff fix(mac): Wrap hditutil detach in retry w/ backoff Jun 11, 2023
@AndrewGable
Copy link

FYI - We tested the fix here and it worked as intended in fixing the error we were seeing earlier. Nice work @roryabraham!

@laurent22
Copy link
Contributor

Unfortunately this fix doesn't help much. We see the script retrying up to 5 times on CI and eventually it still fails with the same error message

@mmaietta
Copy link
Collaborator

mmaietta commented Dec 1, 2023

Pretty sure there's no way to detect this from javascript side. All dmg-builder is doing is calling hdiutil directly via exec/spawn. There's no callback support to observe the corresponding events.

We do try to use detach --force on failure, but even then, it appears the merged fix still does not resolve the issue for all developers.

In a previous project, I had Obj-C code (via a .node module) that tapped into the NSNotificationCenter to observe system-level events of dmg mount/unmounting for native support of dmg auto-updates. I've previously attempted to do the same thing with javascript and it doesn't work 😓

@laurent22
Copy link
Contributor

I wonder if some macOS command line could be used to detect what is preventing the unmount action from happening?

Maybe that could be printed to stdout in failure so that we know what is locking files or folders.

@mmaietta
Copy link
Collaborator

mmaietta commented Dec 2, 2023

@laurent22 can you create a new Github issue for this and ping me there/link it here? I'd rather contain discussions there as I forsee this to be a larger topic and not best suited as comments on a merged PR.

I'm whipping up a patch-package that you can test with for pulling more debug data. 🙂

@roryabraham
Copy link
Contributor Author

roryabraham commented Jun 27, 2024

was another issue ever created? this issue has started coming back to haunt us

@mmaietta
Copy link
Collaborator

@roryabraham I don't think a new one exists. Are you on the same version of electron-builder and the issue came back? Or did you update electron-builder? I recall there's a new retry mechanism for hdiutil that was added about 4 months ago for wrapping the retry for all hdiutil commands: https://github.com/electron-userland/electron-builder/blob/master/packages/dmg-builder/src/hdiuil.ts

@roryabraham
Copy link
Contributor Author

roryabraham commented Jul 17, 2024

We are currently on 24.13.2, we'll try updating to 25.0.0. Thanks 👍🏼

though I haven't really seen this issue in the last few weeks. It showed up relentlessly for a few days then disappeared back into the ether 🤷🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants