Skip to content

Runtime crash on failure to build count fetch request - #47

Open
roanutil wants to merge 4 commits into
mainfrom
runtime-crash-on-failure-to-build-count-fetch-request
Open

Runtime crash on failure to build count fetch request#47
roanutil wants to merge 4 commits into
mainfrom
runtime-crash-on-failure-to-build-count-fetch-request

Conversation

@roanutil

Copy link
Copy Markdown
Owner

Guard against runtime crashes caused by invalid requests

runtime-crash-on-failure-to-build-count-fetch-request
runtime-crash-on-failure-to-build-count-fetch-request
runtime-crash-on-failure-to-build-count-fetch-request
@r-jarvis

Copy link
Copy Markdown
Collaborator

CoreDataErrorNSError erasure still present in Aggregate.send

The fix from 3788d28 ("Fix erasure of CoreDataError to NSError in count endpoint") applies to Self.send too, but that path wasn't updated.

Sources/CoreDataRepository/CoreDataRepository+Aggregate.swift:894-900:

do {
    return try Self.aggregate(context: scratchPad, request: request)
} catch let error as CocoaError {
    throw CoreDataError.cocoa(error)
} catch {
    throw CoreDataError.unknown(error as NSError)   // swallows CoreDataError
}

Self.aggregate (line 859) throws CoreDataError.fetchedObjectFailedToCastToExpectedType(description: nil). It isn't a CocoaError, so the catch-all rewraps it as .unknown(NSError).

Worth noting: performInChild already handles this correctly — it leads with catch let error as CoreDataError { return .failure(error) } — but it never gets the chance, because the inner catch-all has already converted the value before it propagates out of the block.

Net effect: an average / sum / min / max whose result fails asAggregateValue() (e.g. requesting Int from a Double attribute) surfaces as .unknown(NSError domain "CoreDataRepository" code 3) rather than the typed case — the same symptom count had before 3788d28.

Fix is the same one-line clause already added at line 223:

} catch let error as CoreDataError {
    throw error
} catch let error as CocoaError {

runtime-crash-on-failure-to-build-count-fetch-request
@roanutil

Copy link
Copy Markdown
Owner Author

CoreDataErrorNSError erasure still present in Aggregate.send

The fix from 3788d28 ("Fix erasure of CoreDataError to NSError in count endpoint") applies to Self.send too, but that path wasn't updated.

Sources/CoreDataRepository/CoreDataRepository+Aggregate.swift:894-900:

do {
    return try Self.aggregate(context: scratchPad, request: request)
} catch let error as CocoaError {
    throw CoreDataError.cocoa(error)
} catch {
    throw CoreDataError.unknown(error as NSError)   // swallows CoreDataError
}

Self.aggregate (line 859) throws CoreDataError.fetchedObjectFailedToCastToExpectedType(description: nil). It isn't a CocoaError, so the catch-all rewraps it as .unknown(NSError).

Worth noting: performInChild already handles this correctly — it leads with catch let error as CoreDataError { return .failure(error) } — but it never gets the chance, because the inner catch-all has already converted the value before it propagates out of the block.

Net effect: an average / sum / min / max whose result fails asAggregateValue() (e.g. requesting Int from a Double attribute) surfaces as .unknown(NSError domain "CoreDataRepository" code 3) rather than the typed case — the same symptom count had before 3788d28.

Fix is the same one-line clause already added at line 223:

} catch let error as CoreDataError {
    throw error
} catch let error as CocoaError {

fixed

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