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

Propagate I/O errors from filesystem and serialization failures #28

Merged
merged 3 commits into from
Jul 18, 2021

Conversation

bhamiltoncx
Copy link
Contributor

@bhamiltoncx bhamiltoncx commented Jul 14, 2021

This PR fixes #7 .

Previously, I/O and serialization/deserialization errors were silently
swallowed.

This commit makes use of the NSFileHandle APIs on iOS 13 and later to
perform filesystem operations and propagate the errors to the caller.

The following APIs are now deprecated in favor of the variants that take
a (nullable) NSError **:

-[CASQueueFile add:]
-[CASQueueFile peek:]
-[CASQueueFile pop:]
-[CASQueueFile clear]
-[CASObjectQueue add:]
-[CASObjectQueue peek:]
-[CASObjectQueue pop:]
-[CASObjectQueue clear]

In addition, the two convenience methods -peek and -pop to access
a single element are deprecated since there is no way to distinguish
an empty queue from an error:

-[CASObjectQueue peek]
-[CASObjectQueue pop]

Use -peek:error: and -pop:error: instead, and call -firstObject
on the result if you don't need to distinguish an empty queue from an
error.

I also bumped the Azure Xcode version to Xcode 12 so CI builds with the new APIs.

@bhamiltoncx bhamiltoncx force-pushed the propagate-errors branch 3 times, most recently from d7ebdf4 to 5c2e4b7 Compare July 14, 2021 21:24
Copy link
Contributor

@nikhilbedi nikhilbedi left a comment

Choose a reason for hiding this comment

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

Looks great! Left one comment

Cassette/CASQueueFile.m Outdated Show resolved Hide resolved
@bhamiltoncx
Copy link
Contributor Author

Looks great! Left one comment

Thanks again! I think I fixed the issue, but I didn't quite understand this part:

and sometime after successfully resetting the header

Let me know if the updated PR does what you were looking for — I wasn't 100% sure what you meant there.

@bhamiltoncx
Copy link
Contributor Author

Forgot to update CASSampleApp, fixed.

Cassette/CASQueueFile.m Outdated Show resolved Hide resolved
Previously, I/O and serialization/deserialization errors were silently
swallowed.

This commit makes use of the NSFileHandle APIs on iOS 13 and later to
perform filesystem operations and propagate the errors to the caller.

The following APIs are now deprecated in favor of the variants that take
a (nullable) `NSError **`:

```
-[CASQueueFile add:]
-[CASQueueFile peek:]
-[CASQueueFile pop:]
-[CASQueueFile clear]
-[CASObjectQueue add:]
-[CASObjectQueue peek:]
-[CASObjectQueue pop:]
-[CASObjectQueue clear]
```

In addition, the two convenience methods `-peek` and `-pop` to access
a single element are deprecated since there is no way to distinguish
an empty queue from an error:

```
-[CASObjectQueue peek]
-[CASObjectQueue pop]
```

Use `-peek:error:` and `-pop:error:` instead, and call `-firstObject`
on the result if you don't need to distinguish an empty queue from an
error.
@nikhilbedi
Copy link
Contributor

Thanks for doing this! Adding proper error handling has been a long-standing TODO :)

@bhamiltoncx
Copy link
Contributor Author

Glad to help! I have a few more little features coming for you. :)

@nikhilbedi nikhilbedi merged commit 78578e5 into linkedin:master Jul 18, 2021
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.

NSFileHandle iOS 13 Deprecations
2 participants