Crash after Swift 3 update

[andrealufino]

Hello guys, I'm having a serious problem with Pantry. I didn't change anything from what I had before the update (it's a project that could wait). After this update, Pantry crashes at every pack call. I pack arrays (of every type, even custom structs that conforms to the Storable protocol) and other kind of stuff. Before the update everything was working good, not now.

One of the call is simple as :

Pantry.pack(self, key: pantryKeyCurrentUser)

Here crashes pointing me to this line (number 118 of file JSONWarehouse):

let data = try JSONSerialization.data(withJSONObject: storableDictionary, options: .prettyPrinted)

The error is EXC_BAD_INSTRUCTION (code = EXC_I386_INVOP, subcode = 0x0)

[nickoneill]

Thanks, I'll check this out today

[andrealufino]

Do you have some news?

[nickoneill]

Hey @andrealufino, I don't see any specific problems with this line in swift 3. In fact, it should be safer than previously since we wrap those try blocks to catch errors now.

Does this crash on a device as well as the sim?

[andrealufino]

Yes, it crashes on both.
This is the log into the console :
fatal error: attempt to bridge an implicitly unwrapped optional containing nil

[andreamazz]

Hi @andrealufino
Can you reproduce the issue in the sample project to help debug the issue?

[andrealufino]

I tried, but in the example app everything is working good.
In my project this is the piece of code from which everything goes wrong :

var newsLocal: [News] = [] {
        didSet {
            print("News - News local just update! Now we'll save the new array with Pantry!")
            Pantry.pack(newsLocal, key: pantryKeyNewsLocal)
            print("News - Local news saved with Pantry!")
        }
    }

News is a struct which implements the Storable protocol.

Could be that if the array is empty (or nil) something happens? I don't know, but I can assure you this was working really good with the older version (before Swift 3).

[andreamazz]

Empty arrays are packed correctly, and from the code you posted the array can't be nil (unless you use force unwrapping, but in that case it would bail out earlier)

[andrealufino]

Exactly, that array will never be nil. It comes from the server, but even if I have no news to show, I pass an empty array and not a nil one. The weird thing is that fatal error: attempt to bridge an implicitly unwrapped optional containing nil console log.

[andrealufino]

What if some attribute of the a news object is nil?

[andreamazz]

Yeah, that might be the issue as far as I can tell

[andrealufino]

This thing has changed from previous version, right?

[andreamazz]

Yep, this is due to the JSON serialization.

[andreamazz]

Ok, I was able to reproduce the issue. Let me see if I can fix it.

[andreamazz]

The problem is inherent to Swift 3's Any type, which can hold an optional of nil value. The JSON serializer doesn't play well with a nil value.
You can test this in a playground:

let packet: [String: Int?] = ["one": nil]
let dictionary: [String: Any] = ["packet": packet]

do {
    let data = try JSONSerialization.data(withJSONObject: dictionary, options: .prettyPrinted)
} catch {
    print("error")
}

I'm not sure if reverting to the use of AnyObject in the toDictionary func of the Storable protocol might be the better course of action. It will limit the flexibility of Any, but it would provide a safer API. What do you think @nickoneill?

In the meantime @andrealufino you can use nil coalescing in your toDictionary implementation, providing default values for nullable objects, like so:

struct News: Storable {
    var title = ""
    var body: String? = nil

    init?(warehouse: Warehouseable) {}

    func toDictionary() -> [String: Any] {
        return ["title": title, "body": body]
    }
}

[andrealufino]

In my structures there is no toDictionary() method implementation however and I've never needed it. Could this be another problem? I can assure you everything was working good even without it. This is my News struct :

import Foundation
import Pantry

private let keyImage = "image"
private let keyTitle = "title"
private let keyBody = "body"
private let keyDate = "date"
private let keyUrl = "url"
private let keyType = "type"

// The protocols StorableRawEnum and Storable are provided by the Pantry framework.
// These protocols are useful when you have to permanently store data on disk

enum NewsType: Int, StorableRawEnum {
    case local  = 2
    case global = 1
}

struct News: Storable {

    var image: String!
    var title: String!
    var body: String!
    var date: String!
    var url: String!
    var type: NewsType!

    init?(warehouse: Warehouseable) {

        self.image = warehouse.get(keyImage) ?? ""
        self.title = warehouse.get(keyTitle) ?? ""
        self.body = warehouse.get(keyBody) ?? ""
        self.date = warehouse.get(keyDate) ?? ""
        self.url = warehouse.get(keyUrl) ?? ""
        self.type = warehouse.get(keyType) 
    }

    init(title: String!, body: String!, image: String!, date: String!, url: String!, type: NewsType!) {

        self.title = title
        self.body = body
        self.image = image
        self.date = date
        self.type = type
    }
}

ps: in my opinion is better to keep the Any instead of AnyObject for flexibility reasons adding maybe an assert to make people understand what is causing the problem.

[andreamazz]

I agree on the Any vs AnyObject.
There are a couple of related radars open:
SR-2390
SR-2378

For the moment it would be better to check if the JSON is valid with:
JSONSerialization.isValidJSONObject(_:)
and eventually throw.

[andreamazz]

@andrealufino check out branch issue-56 for a fix and let me know.

[andrealufino]

Will do!

[nickoneill]

Thanks for the detailed investigation @andreamazz. I agree that for safety we should check for the ability to create JSON before we actually try it, particularly since it'll crash here and not throw an error (why even both having it throw if you're not going to handle cases like this, Apple?).

If we can, we should work around these issues as well.

We should be able to serialize / deserialize an NSNull object... perhaps the best course of action is to check for nils and replace with NSNull. Can you write a few tests that encapsuate this behavior? Then we can test some solutions.

[andreamazz]

Hey @nickoneill
I've added a test case in the branch issue-56 (please note the test currently passes since I added the check in the write func, comment out 117:120 in JSONWarehouse.swfit to reproduce the issue)

After some digging it looks like that the issue happens when naively implementing toDictionary:

    func toDictionary() -> [String: Any] {
        return ["name": name, "age": age]
    }

This fails. If we use the default implementation provided by Mirror, nil is correctly replaced with NSNull and the JSON is valid.

[nickoneill]

I thought @andrealufino didn't provide a toDictionary method?

[andrealufino]

@nickoneill exactly, I didn't provide it. I still have to check the new branch, hope I can do it tomorrow.

[andreamazz]

Can you post a print of your object's toDictionary call?

[nickoneill]

@andrealufino I'm guessing the new branch will prevent the crash, but it also won't pack your News object. Can you print a detailed list of everything in the News object that crashes the JSONSerializer?

[andrealufino]

This is the News struct :

import Foundation
import Pantry

private let keyImage = "image"
private let keyTitle = "title"
private let keyBody = "body"
private let keyDate = "date"
private let keyUrl = "url"
private let keyType = "type"

// The protocols StorableRawEnum and Storable are provided by the Pantry framework.
// These protocols are useful when you have to permanently store data on disk

enum NewsType: Int, StorableRawEnum {
    case local  = 2
    case global = 1
}

struct News: Storable {

    var image: String!
    var title: String!
    var body: String!
    var date: String!
    var url: String!
    var type: NewsType!

    init?(warehouse: Warehouseable) {

        self.image = warehouse.get(keyImage) ?? ""
        self.title = warehouse.get(keyTitle) ?? ""
        self.body = warehouse.get(keyBody) ?? ""
        self.date = warehouse.get(keyDate) ?? ""
        self.url = warehouse.get(keyUrl) ?? ""
        self.type = warehouse.get(keyType) 
    }

    init(title: String!, body: String!, image: String!, date: String!, url: String!, type: NewsType!) {

        self.title = title
        self.body = body
        self.image = image
        self.date = date
        self.type = type
    }
}

The attribute that is always nil (for now) is the url. In the previous version, I didn't encounter problems, but now, as you know, yes.

[nickoneill]

How about a description of the actual object that is failing to serialize? I didn't even notice all the IUOs before but I'm guessing they're the problem. It doesn't look like they're necessary for your object either - do you know why you're using them?

[andrealufino]

Using the po command I see something like this

- .0 : "isUserFirstAccess"
- .1 : <null>

somewhere in the code. I'm printing the parameter object of the write func in the JSONWarehouse class. The same is printed using the po storableDictionary["storage"].

The isUserFirstAccess is declared in this way into a class :

var isUserFirstAccess: Bool? {
        didSet {
            Pantry.pack(self, key: pantryKeyCurrentUser)
        }
    }

Every time something in that class changes, I pack again the whole object, that's why I wrote the self there.

ps : I can't print the whole object because contains sensitive information (it's not only the News object which crashes, now is this). Do you think could be the nil or the ! in some params?

Basically, Pantry is an essential part of this project and I hope we can make it works :)

[andrealufino]

Do you have a solution? Even temporary, even If I have to change something to make it works with the current Pantry version

[andreamazz]

Let's clear up some confusion. Did you test the issue-56 branch? Did that solve the crash?

[andrealufino]

Hey guys, sorry for the late response. I tried that branch, but the same happened :
fatal error: attempt to bridge an implicitly unwrapped optional containing nil
Same class, same line as before.

[nickoneill]

I didn't even notice all the IUOs before but I'm guessing they're the problem. It doesn't look like they're necessary for your object either - do you know why you're using them?

I'll bring up the same issue as before. Is there a reason for the IUOs that I'm missing? They look unnecessary.

You're going to crash on accessing whatever property is an IUO if you try to access it normally as well. The only difference here is that Pantry is trying to access each of your properties in an attempt to store them so the issue is automatically discovered.

@andreamazz I'm still a fan of checking our valid JSON before we write it. Can you submit a PR from your branch?

[andrealufino]

You're right, however, no, no specific reason, so they can be removed. I think this should be however handled somewhere, because in the old Pantry version everything was good.

[andrealufino]

Hey guys :) Do you have some news about this?

[AntiPavel]

Accidentally saw this issue when I was searching how to solve my similar problem. Eventually I decided it with remove all optional values in ​​serialized dictionary. Maybe it helps.

[nickoneill]

@AntiPavel Specifically IUOs and not just optionals, right?

[AntiPavel]

Yes, right

[nickoneill]

@AntiPavel @andrealufino can you see if the latest master (not a versioned release on cocoapods yet) is a reasonable fix for you?

[andrealufino]

Hi guys, I've just checked the latest master, but always the same. It crashes at line 117, here :

guard JSONSerialization.isValidJSONObject(storableDictionary) else {
    debugPrint("Not a valid JSON object: \(object)")
    return
}

I think it's something added in the last commit. The error is EXC_BAD_INSTRUCTION. This is the object I'm trying to store :

▿ 2 elements
  ▿ 0 : 2 elements
    - .0 : "expires"
    - .1 : 64092211200.0
  ▿ 1 : 2 elements
    - .0 : "storage"
    ▿ .1 : 2 elements
      ▿ 0 : 6 elements
        ▿ 0 : 2 elements
          - .0 : "date"
          ▿ .1 : 2016-10-06
            - some : "2016-10-06"
        ▿ 1 : 2 elements
          - .0 : "title"
          ▿ .1 : Thank you!
            - some : "Thank you!"
        ▿ 2 : 2 elements
          - .0 : "type"
          ▿ .1 : local
            - some : NewsType.local
        ▿ 3 : 2 elements
          - .0 : "image"
          ▿ .1 : tunnelbana.jpg
            - some : "tunnelbana.jpg"
        ▿ 4 : 2 elements
          - .0 : "body"
          ▿ .1 : Thank you for supporting with the testing of the App!
            - some : "Thank you for supporting with the testing of the App!"
        ▿ 5 : 2 elements
          - .0 : "url"
          - .1 : nil
      ▿ 1 : 6 elements
        ▿ 0 : 2 elements
          - .0 : "date"
          ▿ .1 : 2016-09-06
            - some : "2016-09-06"
        ▿ 1 : 2 elements
          - .0 : "title"
          ▿ .1 : " I love watching Dr Phil"
            - some : "I love watching Dr Phil"
        ▿ 2 : 2 elements
          - .0 : "type"
          ▿ .1 : local
            - some : NewsType.local
        ▿ 3 : 2 elements
          - .0 : "image"
          ▿ .1 : kkrr.jpg__750x500_q85_crop_subsampling-2_upscale.jpg
            - some : "kkrr.jpg__750x500_q85_crop_subsampling-2_upscale.jpg"
        ▿ 4 : 2 elements
          - .0 : "body"
          ▿ .1 : karin has helped 57,000 young people receive support.
            - some : "karin has helped 57,000 young people receive support."
        ▿ 5 : 2 elements
          - .0 : "url"
          - .1 : nil

[reinaldoluckman]

In my case, I had one nested class. Let's say class A has a mandatory reference to class B:

class A: Storable {  
    var id: String
    var b: B!
}

The type of the mandatory reference is serialized by Mirror as MyApp_B!. I suspected that the exclamation mark could be a problem, so I removed it.

After that, the problem was gone and my object of type class A is serialized without problem.

Hope this help can help you.

[xiaozao2008]

@andrealufino
my error info
attempt to bridge an implicitly unwrapped optional containing nil

I've solve it. I think maybe helpful for you.
class AA inherit from a class A (class A is an Objective-c class).
@Property (nonatomic, copy, readonly) NSString *conversationId;

in class AA when I do ..
let params = [ "USER_ID":self.peerID,
"CONVERSATION_ID":self.conversationId,
"CONVERSATION_TYPE":self.messageType
]
It crash because sometimes "self.conversationId == nil"
OC is not en enough safety.
in swift it means"!" the property
let params = [ "USER_ID":self.peerID!,
"CONVERSATION_ID":self.conversationId!,
"CONVERSATION_TYPE":self.messageType!
]

same as your project
JSONSerialization.data(withJSONObject: Any, options: JSONSerialization.WritingOptions)
you need sure storableDictionary != nil