Grdb.swift: Feature Request: Failable initializers

[cjarvi]

Hi there. I'd like to request an enhancement to GRDB for your consideration: Failable initializers, specifically in the FetchableRecord protocol. Perhaps something like init?(row:Row)

The use case for this is when storing/retrieving blob objects there's a chance for the Data() conversion to fail, especially when using custom de/serialization. If this failure occurs, I'd like to be able to fail gracefully and not create the object.

I attempted to roll my own, but found that I would need to override quite a few things to make it work...particularly anything that relies on the Record protocol. It just didn't seem worth it at the time

[groue]

Hello @cjarvi,

You're not the first requesting a failable or a throwing initializer. This is quite a valid request! But please follow me:

When a type can't decode a Row, it is often the sign of a database that contains bizarre values. Those databases are called "untrusted databases". They exist, I've met them! And they are not warmly welcomed in GRDB, as you have noticed.

Supporting untrusted database would come with problems that would weigh on the shoulders of all users, including those who deal with trusted databases. Not only a crippled API with more try and if let than ever, but lost opportunities for optimizations that are only possible when decoding is safe (such as deferred decoding, a technique used by some database observation tools).

A local database is not some JSON loaded from a remote server. The database is right there on your device: it is your thing.

This is why GRDB fosters trusted databases, and will continue doing so for the foreseeable future, for the benefit of all.

Now let's bring some possible solutions to your issue :-)

---

First option: Clean the database

The use case for this is when storing/retrieving blob objects there's a chance for the Data() conversion to fail, especially when using custom de/serialization

In your particular cases, it looks like migrations may well be your friends.

Before your record types can even think about fetching anything, give the database a good massage with a migration that decides what should be done with those invalid blobs: repair, destroy, mark them as invalid in a dedicated column. But don't let the migration quit until the database is in a pristine and controlled state.

After that, your records will be able to adopt FetchableRecord without a second thought, and this will greatly simplify your application logic.

---

Second option: Let record types decode invalid rows

Keep the database in a dirty state, but make your records able to deal with it: have them tell if they could decode their database row, or not. Here is a quick simplified example:

struct DatabaseUser: FetchableRecord {
    enum Content {
        case invalid
        case valid(name: String)
    }
    var content: Content

    init(row: Row) {
        if let name = row["name"] as String? {
            content = .valid(name)
        } else {
            content = .invalid
        }
    }
}

After you have loaded those records, you'll be able to apply any further decoding logic, such as the failable initializer you are requesting:

struct User {
    var name: String
}

extension User {
    init?(_ record: DatabaseUser) {
        guard case let .valid(name) = record.content else {
            return nil
        }
        self.init(name: name)
    }
}

let dbUsers: [DatabaseUser] = try DatabaseUser.fetchAll(db, ...)
let users1: [User?] = dbUsers.map { User($0) }
let users2: [User] = dbUsers.compactMap { User($0) }

This is a technique I like to apply when I have to decode wacky remote JSON.

---

Third option: To the hell with FetchableRecord!

I know that some apps really need to escape FetchableRecord. Check the Beyond FetchableRecord documentation chapter.

As you'll see, this is a more involved scenario.

---

Whatever the option you choose, don't hesitate asking more questions!

[groue]

No news is good news. Happy GRDB, @cjarvi!

[DnV1eX]

Hello @groue,
I understand and recognize your position, but could you please clarify, how exactly the inclusion of a separate FailableFetchableRecord protocol in the framework can hurt "law abiding" adepts of "trusted databases"? They might just not use it.
I mean, all these functions such as fetchAll already throw SQLite errors. I am not talking about special behavior like skipping corrupted records, default implementation for FailableFetchableRecord could simply rethrow its decoding errors.

[groue]

Hello @DnV1eX,

The Obvious, the Easy, and the Possible had a tremendous influence on me. I'm constantly dispatching GRDB features in the "obvious", "easy" and "possible" buckets.

The reason why this idea is so efficient for libraries is that it acknowledges that we have limited resources to deal with. For example, the complexity of the mental model in users's head should stay as low as possible.

On this regard, the inclusion of a separate FailableFetchableRecord protocol brings several problems:

• It introduces one more need for an informed choice on the path to a working application. And every choice adds one layer of complexity, and one opportunity for a mistake.
• The current non-failable FetchableRecord protocol grants all GRDB errors with a notable property: they all are I/O errors, or application bugs that require a change in the code or the database schema. No coworker of yours can introduce a failable record behind your back which would break this rule. I/O errors are very different from invalid data errors. Many applications can bail out (try!) on database errors, especially on iOS, watchOS, and tvOS, where application users can not mess with the file system.

Now, FailableFetchableRecord could ship in an external library. Its added complexity would only bother users who want to deal with it.

FailableFetchableRecord belongs to the "possible" bucket. In all honesty, I know it is not fully there yet: for example, RowDecoder is not public, and this makes it difficult to derive FailableFetchableRecord from Decodable (a reasonable expectation). If someone eventually wants to develop FailableFetchableRecord, I expect feature requests of this kind ("Please make RowDecoder public"). But FailableFetchableRecord will not be merged in GRDB itself.

[DnV1eX]

@groue, thanks for the detailed response!

I had a chance to explore the framework in more detail. Now I realize that "untrusted databases" more than just not warmly welcomed, they are _really_ not supported. I believe it would be fair to mention this "small" fact somewhere at the beginning of the documentation, because this ideological limitation is actually more significant than you tend to think. It is generally accepted that no code (and especially no third-party framework) should trust user data so much to crash the released app. Even SQLite library itself trying hard to convert misrepresented data. Nowadays Swift has powerful exceptions and I'm sad that you're avoiding using them in this case, while willingly adopting other new language features.

[groue]

Now I realize that "untrusted databases" more than just not warmly welcomed, they are really not supported.

I put it in a very different frame. Untrusted database are supported, but they require more work for the developer. This work includes code, but also thinking about the best way to deal with the mess.

GRDB is a pile of layers, which starts from database file, climbs to SQL queries, then Record protocols, and finally Codable Records. When one layer does not fit your needs, you can always go down one level and find your solution there.

It is generally accepted that no code (and especially no third-party framework) should trust user data so much to crash the released app.

It may help to consider the database not as user data. but as application data. Check SQLite As An Application File Format, for example. As I like to say, don't look at your local SQLite database as you look at the JSON you load from a remote server. You can't trust the JSON. On the other side, the SQLite database is local: you can put it on your side, make it trustable.

Nowadays Swift has powerful exceptions and I'm sad that you're avoiding using them in this case, while willingly adopting other new language features.

i understand. I'm also sad when I have to deal with wacky data.

Feel free to ask a precise question eventually. Theorical conversations like this one give an interesting context, but they don't quite help moving forward to actual solutions.

[chrisballinger]

@groue I was about to make a new issue about support for graceful fallback (to nil, or otherwise) for Codable blobs that can no longer be decoded, but it seems there is existing discussion here. Let me know if I should create a new issue instead for continued discussion.

My use case here is storing loosely structured data that can be easily refetched, so I don't mind if these blobs end up nil occasionally when there has been a change to the Codable structure.

What would be the minimum viable approach for a patch that you'd consider merging to support this workflow? It seems like maybe RowDecoder would need to be made public as you mentioned, and the rest of the implementation could live in another external package. I'd like to avoid as much duplication as possible, especially for the much simpler use case of just making optional Codable properties go to nil when they fail to decode.

[groue]

Hello @chrisballinger,

I will start with a "negative" feedback, in an effort to express some hard limits. I'll open doors just after.

I was about to make a new issue about support for graceful fallback (to nil, or otherwise) for Codable blobs that can no longer be decoded [...]
My use case here is storing loosely structured data that can be easily refetched, so I don't mind if these blobs end up nil occasionally when there has been a change to the Codable structure.

There will never be any "graceful fallback". Either rows are decoded, or not. When they are not, some error must be reported.

The reason for this is that GRDB will never silently lose data. Data loss is the responsibility of applications.

Is there any "graceful fallback" in the standard Decodable protocol? No there isn't: apps have to implement their custom lossy decoding.

Currently, GRDB only raises fatal errors when decoding fails.

The feature request we can talk about is that decoding failures are reported as catchable errors instead of fatal errors.

The good news is that I now support this feature request 😄. The main blocking point, the "deferred decoding" that was still used in GRDB 4 with FetchedRecordsController, has become moot since GRDB 5. The path has been cleared.

---

Mandatory reminder: you can avoid fatal errors today.

Just decode values that can not fail at decoding, namely raw database rows. This gives something which is verbose (but works):

// No longer FetchableRecord
struct Player: TableRecord {
    var name: String
    var score: Int
}

extension Player {
    struct DecodingError: Error { }

    init(row: Row) throws {
        guard let name = String.fromDatabaseValue(row["name"]),
              let score = Int.fromDatabaseValue(row["score"])
        else { throw DecodingError() }

        self.init(name: name, score: score)
    }
}

// No crash
let request = Player.all()
let rows = try Row.fetchAll(db, request)
let players: [Player] = rows.compactMap { row in try? Player(row: row) }

You can even define your own FailableFetchableRecord protocol in your app:

protocol FailableFetchableRecord {
    init(row: Row) throws
}

extension Player: FailableFetchableRecord { }

extension QueryInterfaceRequest where RowDecoder: FailableFetchableRecord {
    func fetchAll(_ db: Database) throws -> [RowDecoder] {
        try Array(Row.fetchCursor(db, self).map(RowDecoder.init(row:)))
    }
}

// No crash
let request = Player.all()
let players = try request.fetchAll(db)

The code above compiles today, and works.

This is not convenient, of course.

---

What would be the minimum viable approach for a patch that you'd consider merging to support this workflow?

Thanks for asking 😅

The minimum viable approach is... involved.

We're talking about:

// Throws an error
let date = try Date.fetchOne(db, sql: "SELECT 'yesterday' AS date")

// Throws an error
struct Player: FetchableRecord { ... )
let player = try Player.fetchOne(db, sql: "SELECT NULL AS name")

First, those errors should contain as much information as the current fatal errors (this is so useful for debugging, and in crash reports):

// Fatal error: could not convert database value "yesterday" to
// DatabaseDateComponents (column: `date`, column index: 0,
// row: [date:"yesterday"], sql: `SELECT 'yesterday' AS date`)
let date = try Date.fetchOne(db, sql: "SELECT 'yesterday' AS date")

// Fatal error: could not convert database value NULL to String
// (column: `name`, column index: 0, row: [name:NULL],
// sql: `SELECT NULL AS name`)
let player = try Player.fetchOne(db, sql: "SELECT NULL AS name")

I wonder if decoding Decodable records should always throw DecodingError. Even so, we must not lose the debugging info.

(Side note: I'm a little uncomfortable because the GRDB code that generates this debugging info is really not very nice.)

Second, if applications can start dealing with untrusted database by catching decoding errors, the life must not become harder for apps that don't care crashing when the database content does not match the code. In particular, I won't get rid of subscripts easily:

// Decoding values
let date: Date = row["date"] // (can fatal error)
let date = try ... // must invent a new throwing API

// Decoding to-one associations (see Association Guide, and RowAdapter)
let subRecord: Player = row["player"] // (can fatal error)
let subRecord = try ... // must invent a new throwing API

// Decoding to-many associations (see Association Guide)
let subRecords: [Player] = row["players"] // (can fatal error)
let subRecords: Set<Player> = row["players"] // (can fatal error)
let subRecords = try ... // must invent a new throwing API

Those subscripts are used. Some people from the "FMDB crowd" do not quite trust/grok record types and the query interface, and stay at the SQL level. Some people do not use the Decodable protocol (because of its performance overhead, or whatever other reason).

Third, I remain convinced that decoding errors are most often programmer errors that require a fix (not another catch block). For example, either the database should add a missing NOT NULL constraint, either a Swift struct must change its property to some optional type. I have a big difficulty putting a local and controlled database in the same bag as an untrustable remote server. So much code is written in order to deal with lousy server JSON. If developers start writing similar defensive code when they access their local database, something rare, great, slightly educational, and sane (because fatal errors while debugging lead to less bugs in released apps) in this library will be lost.

---

Let's look at the three fundamental decoding protocols: DatabaseValueConvertible, StatementColumnConvertible and FetchableRecord.

• DatabaseValueConvertible already support decoding failures, by returning nil from its factory method.
• StatementColumnConvertible does not support decoding failures (such as decoding Int8 from 256, or Date from yesterday).
• FetchableRecord does not support decoding failures.

Can we make the StatementColumnConvertible and FetchableRecord throwing without introducing breaking changes in applications?

StatementColumnConvertible is key for performance, but is barely public/documented. It is unlikely that any GRDB-powered app uses it directly.

FetchableRecord, on the other side, is in every app. That's risky. The doc even contains sample code (example) that would break if its initializer would become throwing:

// An observation of distinct [TeamInfo]
let request = Team.including(all: Team.players)
let observation = ValueObservation
    .tracking { db in try Row.fetchAll(db, request) }
    .removeDuplicates()
    .map { rows in rows.map(TeamInfo.init(row:) } // Would stop compiling

So it looks like we're aiming at GRDB 6, with a breaking change on FetchableRecord.

[groue]

@chrisballinger: if this becomes serious in a way or another, a new issue will be welcomed indeed.

[chrisballinger]

@groue Thank you for the detailed response! To be more specific about this use case, here's a snag I hit recently:

public struct PersistedModel<T: Codable>: Codable, Identifiable, FetchableRecord, MutablePersistableRecord {
    public var id: Identifier<T>
    public var somethingCodable: T?
}

public struct Example: Codable {
    public var someField: String?
}

let example = try PersistedModel<Example>.fetchOne(db)

public struct Example: Codable {
-   public var someField: String?
+   public var someField: Date?
}

In this case I've changed the Example model in a way that no longer decodes if we've already persisted data. Since this data can be easily re-fetched for my use case, the simplest approach for me would be to just silently nil out the PersistedModel<Example>.somethingCodable field and possibly print a warning to the console, instead of crashing the app. I agree that Decodable failures should ideally be reported back up to the application layer to be handled / potentially recovered, but it's definitely a larger undertaking and a breaking change as you mentioned.

For a minimum viable patch that you would not merge to accomplish this behavior, where should I be looking inside FetchableRecord+Decodable.swift?

Or maybe it would be easier to just wipe the whole database?

migrator.eraseDatabaseOnDecodableFailure = true

Either way I just want to easily nuke things during early development and carry on with my day, instead of having to deal with a hard crash that requires deleting and reinstalling the app.

[groue]

@chrisballinger, it looks like the old strings that can't be decoded as Date will never be decoded again, and even that you are ready to wipe out the database. Why bother patching FetchableRecord and make it unreliable for the whole app (data loss 👿), when a few lines in the app can wipe out the database or set the improper dates to NULL, so that code and db remain in sync?

[chrisballinger]

@groue In this case we are early in development so I'd prefer to just nuke things similar to the approach taken here:

migrator.eraseDatabaseOnSchemaChange = true

I do have debug menu items for manually wiping the database in the UI, but the app crashes here before I can get there, because there are some db observers set up pretty early.
https://github.com/groue/GRDB.swift/blob/bc5ba1fda57577623f7b7ab617958e0163a3b186/GRDB/Record/FetchableRecord%2BDecodable.swift#L7

The only option at that point is to delete and reinstall the app, which is a frustrating development experience. I don't want to always be wiping the database on first launch with a special code path, and I'm not trying to write migrations for structures that aren't solidly defined yet.

Once things are settled down a bit as far as our models and schema, it'd be better to handle these failures more strictly, but I don't think it makes sense at early stages of development.

[groue]

OK, now I understand what you're after 😅

The only option at that point is to delete and reinstall the app, which is a frustrating development experience.

Indeed, and that's where eraseDatabaseOnSchemaChange is handy, we agree.

I don't have any solution for performing the same kind of db nuking when a decoding error happens, unfortunately.

Decoding errors happen without any database reference, so we don't know which database to erase. You can even trigger decoding errors without any database at all:

// No database exists, and yet fatalError:
// nil does not fit non-optional name property
Player(row: ["name": nil])

You can alter GRDB code base and post a notification, through NotificationCenter, right before the fatalError. Your app can catch this notification and erase the database.

[chrisballinger]

@groue Glad we're on the same page! For the NotificationCenter approach, I don't see how erasing the database would prevent the fatalError from happening because once the notification is posted, it's too late, and the app is going to crash no matter what on that code path.

It seems like the only way we could handle this correctly would be to have a throwing version, and use that further up the chain where the db is present:

```swift
extension FetchableRecord where Self: Decodable {
public init(maybeRow row: Row) throws {
self = try RowDecoder().decode(from: row)
}

public init(row: Row) {
    // Intended force-try. FetchableRecord is designed for records that
    // reliably decode from rows.
    self = try! self.init(maybeRow: row)
}

}

[groue]

I don't see how erasing the database would prevent the fatalError from happening

Of course it does not.

It seems like the only way we could handle this correctly would be to have a throwing version

Yes. And the throwing version was discussed there: https://github.com/groue/GRDB.swift/issues/437#issuecomment-764898436

I'm still not sure we are on the same page. I don't understand at all how you expect your app to recover from a nuked database right in the middle of its regular operations. Maybe your app can. But most apps would have to recreate the database and perform complex error handling. There is no such thing as a one-size-fits-all emergency program! We're no longer talking about something that helps during app development.

Please gather your thoughts, and suggest a sane plan if you expect support from GRDB. Otherwise, deleting and reinstalling the app remains the only available option (during that development phase where the database contains obsolete and invalid data). Thank you!

[carltongibson]

I don't see how erasing the database would prevent the fatalError from happening

Of course it does not.

IIUC it wouldn't stop the crash, but the DB would get deleted, so you could then relaunch the app again. This might be a sufficient improvement in DX? 🤔

[chrisballinger]

@groue I don't expect support, as I'm quite familiar with the thankless job of maintaining popular open source libraries. I appreciate your responsiveness and our dialogue so far.

I think for now I will move forward with solutions that fit my own needs, with the expectation that you are unlikely to merge a future pull request changing the current behavior. I wish you well on your maintenance journey.

[groue]

Thanks @chrisballinger !

merge a future pull request changing the current behavior

I am very likely to merge a pull request that would help a large number of GRDB users. I admit I could not quite foresee what it could be after our conversation. Many questions remain open. What we agree on is that when the database schema is in flux, the developer experience could be improved. I value developer UX a lot, so I'm very curious about ideas you could come up with.