fix: replace panic with error return in archive function#705
Conversation
Changes the archive() function signature to return an error instead of panicking on unsupported object types. Propagates the error through all internal callers (serializeArray, serializeMutableArray, serializeSet, serializeMap) and the external caller in objectivec_classes.go. Fixes danielpaulus#179
danielpaulus
left a comment
danielpaulus
left a comment
There was a problem hiding this comment.
Reviewed the panic-to-error conversion and pushed a maintainer follow-up that adds regression coverage for unsupported archive input, including nil. Local verification passed: go test ./ios/nskeyedarchiver, go test ./..., and make build GO_IOS_BINARY_NAME=/tmp/go-ios-pr705-build NCM_BINARY_NAME=/tmp/go-ncm-pr705-build. GitHub currently reports no checks for this fork branch.
|
Thanks @danielpaulus! Returning an error instead of panicking in archive should be friendlier to embedders. |
Yes indeed! Go-ios should never panic! Thanks for fixing it 👍👍 |
|
Thanks for the merge, @danielpaulus. Archive errors propagating instead of panicking should make automation around it much calmer. |
2 participants
Summary
Replaces the
panic()inarchive()with a proper error return. The function now returns([]interface{}, plist.UID, error)instead of panicking on unsupported object types.Why this matters
Issue #179 flags
panic()usage in library code. The maintainer pointed toarchiver.go:76as a good starting PR. Panics in library code crash the calling application instead of letting it handle the error gracefully.Changes
ios/nskeyedarchiver/archiver.go: Changedarchive(),serializeArray(),serializeMutableArray(),serializeSet(), andserializeMap()signatures to return error. Replaced panic at line 83 withfmt.Errorf.ios/nskeyedarchiver/objectivec_classes.go: Updated all callers ofarchive()to propagate errors.Testing
./ios/nskeyedarchiver/...and./ios/...Fixes #179
This contribution was developed with AI assistance (Codex).