Prevent keyNotFound error with unknown simulators #261

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

Jump to bottom

Open

dnicolson wants to merge 4 commits into XcodesOrg:main

from dnicolson:fix-keynotfound-error

Open

Prevent keyNotFound error with unknown simulators #261

dnicolson wants to merge 4 commits into XcodesOrg:main from dnicolson:fix-keynotfound-error

Conversation

@dnicolson

Copy link

@dnicolson dnicolson commented Jan 4, 2023

This pull request prevents the following error when running xcodes runtimes when an "Unknown Platform Simulator" is installed:

Error: keyNotFound(CodingKeys(stringValue: "build", intValue: nil), Swift.DecodingError.Context(codingPath: [_JSONKey(stringValue: "820783CB-C389-4006-B2D2-D063F3FD10A1", intValue: nil)], debugDescription: "No value associated with key CodingKeys(stringValue: \"build\", intValue: nil) (\"build\").", underlyingError: nil))

If one accidentally runs xcrun simctl runtime add with an incompatible pre-iOS 16 Simulator runtime:

xcrun simctl runtime add com.apple.pkg.iPhoneSimulatorSDK15_0-15.0.1.1633542405.dmg

The following error will be displayed:

D: 94DEF0B1-1585-4D71-AB37-98AB08FDD000 <unknown platform> (Unusable - Missing Signature: Error Domain=SimDiskImageErrorDomain Code=3 "Missing Signature" UserInfo={NSLocalizedDescription=Missing Signature, unusableErrorDetail=})

The problem is that it's not immediately obvious that Xcode has still installed the runtime, but with a missing signature state:
PixelSnap 2023年01月04日 at 07 50 16@2x

And when xcodes runs xcrun simctl runtime list -j, there is no build key resulting in the error:

"820783CB-C389-4006-B2D2-D063F3FD10A1" : {
 "deletable" : true,
 "identifier" : "820783CB-C389-4006-B2D2-D063F3FD10A1",
 "kind" : "Disk Image",
 "path" : "\/Library\/Developer\/CoreSimulator\/Images\/Inbox\/820783CB-C389-4006-B2D2-D063F3FD10A1.dmg",
 "signatureState" : "Missing",
 "sizeBytes" : 5304795932,
 "state" : "Unusable",
 "unusableErrorDetail" : "Error Domain=SimDiskImageErrorDomain Code=3 \"Missing Signature\" UserInfo={NSLocalizedDescription=Missing Signature, unusableErrorDetail=}",
 "unusableErrorMessage" : "Missing Signature",
 "unusableSubstate" : "Missing Signature"
}

This change results in the following xcodes runtimes output:

-- Unknown --
Unknown 0 (Downloaded)

Another approach could be to filter out undecodable runtimes in the RuntimeList class, this actually resulted in more code though.

@dnicolson


 Prevent keyNotFound error with unknown simulators

7aa5a38

@dnicolson dnicolson requested a review from a team as a code owner

January 4, 2023 07:27

@StevenSorial

Copy link

Contributor

StevenSorial commented Jan 6, 2023

Nice catch, thank you :)

Another approach could be to filter out undecodable runtimes in the RuntimeList class, this actually resulted in more code though.

I prefer that approach. it doesn't have to be a lot of code, actually, it's less code.

Remove all the installed runtimes with the state unusable, before looping over them, then you can then force-unwrap build, version, and platformIdentifier (and remove the unknown enum case).

I would also add a note at the end with how many unknown runtimes were found, with a suggestion to run xcrun simctl runtime list -j for more info.

Disclaimer: I contributed the runtimes feature but I'm not part of the team, so you might want to wait for their opinion/review.

@dnicolson

Copy link

Author

dnicolson commented Jan 6, 2023

The amount of unknown runtimes is indicated by how many "Unknown" runtimes are listed, it should be comparable to what is in Xcode. It's not ideal that there is a 0 though, but further hacks wouldn't have been nice.

The original approach seemed to require using a custom decoder and possibly filtering out the items unable to be decoded afterward. Or do you mean keeping the optional keys so that the decoding doesn't fail?

@StevenSorial

Copy link

Contributor

StevenSorial commented Jan 6, 2023

Or do you mean keeping the optional keys so that the decoding doesn't fail?

yeah, what i meant is removing the

-- Unknown --
Unknown 0 (Downloaded)

and replacing it at the end with (if more than 0):

Note: Found 1 unknown downloaded runtime(s). For more info run `xcrun simctl runtime list -j`.

you just have to add let unusableCount = installed.removeAll { 0ドル.state == "Unusable" }.count before looping over the installed array, and then you can force-unwrap the newly optional values.

@dnicolson


 Remove unusable runtimes from output

9fb309c

@dnicolson

Copy link

Author

dnicolson commented Jan 6, 2023

I've made those changes, thanks for the input!

MattKiazyk

MattKiazyk requested changes

Jan 6, 2023

View reviewed changes

Copy link

Contributor

@MattKiazyk MattKiazyk left a comment

Choose a reason for hiding this comment

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

Thanks for adding this @dnicolson 🎉

Can we remove the force unwraps? As well as add a quick test for these? Let me know if you need some help.

Sources/XcodesKit/RuntimeInstaller.swift

installed.forEach { runtime in

let resolvedBetaNumber = downloadablesResponse.sdkToSeedMappings.first {

0ドル.buildUpdate == runtime.build

0ドル.buildUpdate == runtime.build!

Copy link

Contributor

@MattKiazyk MattKiazyk Jan 6, 2023

Choose a reason for hiding this comment

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

Can we refactor this section a bit?

Can we not use force unwrap. As a rule, I don't use them. Totally agree that it should never happen but it's bitten me in the past. Perhaps making an overloaded PrintableRuntime() to allow optionals?

Copy link

Author

@dnicolson dnicolson Jan 6, 2023

Choose a reason for hiding this comment

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

I'm not sure how best to achieve this, feel free to make any changes.

Copy link

Contributor

@StevenSorial StevenSorial Jan 8, 2023

Choose a reason for hiding this comment

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

Just my 2 cents here, personally I also try to avoid them if I can, but in this case, I would prefer a force unwrap to fail than hiding the error and providing an empty string as a default value, in that case, we would know that xcrun simctl runtime list printed something that we didn't account for, and there is probably a new state of a runtime that we should write code for. I see it as a assert/precondition and not a code smell.

Copy link

Author

@dnicolson dnicolson Apr 2, 2023

Choose a reason for hiding this comment

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

@MattKiazyk can you please give an update on how you would like to proceed here?

Sources/XcodesKit/RuntimeInstaller.swift

}

Current.logging.log("\nNote: Bundled runtimes are indicated for the currently selected Xcode, more bundled runtimes may exist in other Xcode(s)")

if unusableCount > 0 {

Copy link

Contributor

@MattKiazyk MattKiazyk Jan 6, 2023

Choose a reason for hiding this comment

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

I like this vs having the original Unknown 👍

Sources/XcodesKit/Models+Runtimes.swift Outdated

public struct InstalledRuntime: Decodable {

let build: String

let build: String?

Copy link

Contributor

@MattKiazyk MattKiazyk Jan 6, 2023

Choose a reason for hiding this comment

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

#picky can we add a comment on why these are optional

dnicolson added 2 commits

January 6, 2023 21:07

@dnicolson


 Add comments for unusable runtime attributes

01be5cf

@dnicolson


 Add test for unusable runtimes

dbb6855

@bobbyphtr

Copy link

bobbyphtr commented Jul 24, 2024

Hi, is there any updates for this PR?

@dnicolson

Copy link

Author

dnicolson commented Jul 28, 2024

@MattKiazyk can you please give an update?

Labels

None yet

4 participants

@dnicolson @StevenSorial @bobbyphtr @MattKiazyk

Prevent keyNotFound error with unknown simulators #261

Are you sure you want to change the base?

Prevent keyNotFound error with unknown simulators #261

Uh oh!

Conversation

@dnicolson dnicolson commented Jan 4, 2023

Uh oh!

StevenSorial commented Jan 6, 2023

Uh oh!

dnicolson commented Jan 6, 2023

Uh oh!

StevenSorial commented Jan 6, 2023

Uh oh!

dnicolson commented Jan 6, 2023

Uh oh!

@MattKiazyk MattKiazyk left a comment

Choose a reason for hiding this comment

Uh oh!

@MattKiazyk MattKiazyk Jan 6, 2023

Choose a reason for hiding this comment

Uh oh!

@dnicolson dnicolson Jan 6, 2023

Choose a reason for hiding this comment

Uh oh!

@StevenSorial StevenSorial Jan 8, 2023

Choose a reason for hiding this comment

Uh oh!

@dnicolson dnicolson Apr 2, 2023

Choose a reason for hiding this comment

Uh oh!

@MattKiazyk MattKiazyk Jan 6, 2023

Choose a reason for hiding this comment

Uh oh!

@MattKiazyk MattKiazyk Jan 6, 2023

Choose a reason for hiding this comment

Uh oh!

bobbyphtr commented Jul 24, 2024

Uh oh!

dnicolson commented Jul 28, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants