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

Don't error if our conditional GET fails. #176

Merged
merged 2 commits into from
Jun 15, 2016
Merged

Conversation

joshaber
Copy link
Contributor

@joshaber joshaber commented Jun 14, 2016

This was caused by #175 and #171.

If we tried to download an update and our conditional GET says we already have it, we’d throw an error later when we tried to find the app bundle that we didn't download:

return [RACSignal error:[NSError errorWithDomain:SQRLUpdaterErrorDomain code:SQRLUpdaterErrorMissingUpdateBundle userInfo:userInfo]];

That meant we wouldn’t end up in the SQRLUpdaterStateAwaitingRelaunch state, and so we’d prune the already downloaded update, and still experience #174.

@joshaber
Copy link
Contributor Author

I shoehorned this into an app and it actually does what it should 🎉

NSLog(@"Error removing temporary download directory at %@: %@", downloadDirectory, error.sqrl_verboseDescription);
}
};

return [[[self
downloadBundleForUpdate:update intoDirectory:downloadDirectory]
Copy link
Member

Choose a reason for hiding this comment

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

Is there any way we could have the download signal return the already downloaded URL from the cache to save these if nil branches?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What we really need is a monad transformer for RAC.

But anyways, I like that idea, but essentially all the branches are avoiding doing things with the zip, which we understandably don't keep around after unzipping.

Copy link
Member

Choose a reason for hiding this comment

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

What we really need is a monad transformer for RAC.

Tell me more :thinking_face:

But anyways, I like that idea, but essentially all the branches are avoiding doing things with the zip

Hmm that’s a good point, can we structure it so that we return a signal from inside the conditional get that is flattened, then we can branch on whether we have a zip to decompress or can fulfill the unarchived update from the cache?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reorganized so we can avoid all the annoying nil checks. We're still providing a nil bundle because we want to distinguish between the case where we just downloaded the update and want to verify it, and the case where we already have it and just need to clean up the temp directory.

@keithduncan
Copy link
Member

👍 this looks good to me

@joshaber joshaber merged commit e9e2188 into master Jun 15, 2016
@joshaber joshaber deleted the fix-updates-again branch June 15, 2016 15:05
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