contentful / contentful.swift

A delightful Swift interface to Contentful's content delivery API.
MIT License
202 stars 82 forks source link

Contentful API breaks Swift Concurrency by calling completion handlers more than once #364

Closed tristan-warner-smith closed 1 year ago

tristan-warner-smith commented 2 years ago

Using the Contentful client SDK results in app crashes when completion-based calls fail to decode.

I just encountered this because I've wrapped fetchArray(of in a Task and this triggers Fatal error: SWIFT TASK CONTINUATION MISUSE: fetchArray(of:) tried to resume its continuation more than once, throwing Unknown error occured during decoding.!

The issue I saw is in Client.swift handleJSON<DecodableType: Decodable> where if there's a decode failure the completion is called twice with potentially different errors. This seems to be related to a desire to churnLinks regardless of the failure - but if it failed to decode anyway, we're done there's no value in resolving other failures, we can just go fix our issue and move on, right?

If a failure happens on the original decode the failure completion is called but not returned so it continues to a second decoded check that fails and again calls completion(.failure(error).

The two completion calls are in Client.swift:476 and 500 in Contentful 5.5.2.

This is just how I came across it but any decoding error should reproduce:

nilsen340 commented 1 year ago

Will this be taken care of in the foreseeable future?

tristan-warner-smith commented 1 year ago

@nilsen340 there seems to be no active triage of issues in this repo, my report is more than a year old so I'd assume not.

That said, I created a PR to fix it over here and it was approved and closed, I didn't check to ensure it made it into master though, but I assume it was so YMMV.

nilsen340 commented 1 year ago

@tristan-warner-smith thanks Tristan, it looks like your PR was approved but not merged, it was closed. But it seems they merged a fix later on their own: https://github.com/contentful/contentful.swift/pull/370/files

Not sure if this covers all the cases.

tristan-warner-smith commented 1 year ago

That's enough for me, thanks for checking @nilsen340.