[register_device] fix regression that was causing a crash when creating a device that already exists #17799
Conversation
| UI.verbose("UDID '#{udid}' already exists - Skipping its creation...") | ||
| return existing | ||
| end | ||
|
|
||
| resp = client.post_device(name: name, platform: platform, udid: udid) |
There was a problem hiding this comment.
So I don't know a lot about fastlane's inner workings or best practices, but could another approach be to check/catch if this POST request returns a 409 Conflict and simply ignore it (well, maybe log a message that the device already exists like you're doing above), since it would mean the device already exists. Just thinking that might be a simpler solution and shave off some time from the register_device operation.
But like I said, I don't know a lot about fastlane development, so if what you have works, great!
There was a problem hiding this comment.
I copied the same approach used in the old (now deprecated) method that did the same thing using the iTunes undocumented APIs: instead of recovering from an error after sending the request, it fails the request early before sending it to the server. The server could still return a 409 if a) the local check fails or b) if the request is invalid for other reasons, but I think that's okay
|
@rogerluan I tested this branch and it works. Thanks! Can this be merged now or is there anything I can do to help? |
|
Hey @crooke, thanks for testing this branch and confirming it works! The only thing left is a final ✅ from @joshdholtz 🙏 thanks for your understanding! |
There was a problem hiding this comment.
Congratulations! 🎉 This was released as part of fastlane 2.172.0 🚀
Checklist
bundle exec rspecfrom the root directory to see all new and existing tests passbundle exec rubocop -ato ensure the code style is validMotivation and Context
This PR resolves a regression introduced in #17426
It resolves #17591 and also this user's issue #17429 (comment) (they're the same).
Description
The root cause of this issue is that since #17426 the new
ConnectAPI's "create device" method doesn't check if there's an existing device for the given UDID like thePortal's one used to. This PR adds that verification, and adds tests around it.I couldn't figure out how to create tests for the "create" method 🙇♂️🤔 so I wrote tests only for the new function I created.
Testing Steps
To test this branch, modify your Gemfile as:
Then run
bundle exec fastlane run register_devicepassing a UDID that is already registered.