-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FunctionsError
#13601
FunctionsError
#13601
Conversation
* Introduced `FunctionsError`, a struct that encapsulates information about errors that occur during functions’ execution * Since `FunctionsError` conforms to `CustomNSError`, it transparently bridges to `NSError` when needed; otherwise, it stays a Swift struct (with value semantics and no heap-memory allocation) * Refactored `init(httpStatusCode:body:serializer:)` (fka `errorForResponse(status:body:serializer:)`) to use value types when they’re sufficient (e.g. `[String: Any]` instead of `NSDictionary`) * Introduced `FunctionsErrorTests`
XCTAssertEqual(nsError.localizedDescription, "INTERNAL") | ||
XCTAssertEqual(nsError.userInfo.count, 1) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One use case left untested is the undecodable error details, discussed here.
The problem is, JSONSerialization
supports only a number of types as values, and FunctionsSerializer
can handle all of these types. I couldn’t find a way to construct an error response which triggers FunctionsSerializer
to fail but doesn’t trigger decoding to fail sooner.
// Currently, `internal` is used as the fallback error code. Is this correct? | ||
// Seems like we could get more information from the HTTP status code in such cases. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, this behavior results in data loss. And in general, internal
seems somewhat confusing for unknown error codes (especially when there’s a separate unknown
error case). I guess this should be looked into someday.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. This is fine as is, but maybe the unknown
case should have an associated value to provide a status code with more info. I'm not sure how if it would affect the ObjC interface. In practice, this may not be a rare/non issue.
Thanks, @yakovmanshin! |
FunctionsError
, a struct that encapsulates information about errors that occur during functions’ executionFunctionsError
conforms toCustomNSError
, it transparently bridges toNSError
when needed; otherwise, it stays a Swift struct (with value semantics and no heap-memory allocation)init(httpStatusCode:body:serializer:)
(fkaerrorForResponse(status:body:serializer:)
) to use value types when they’re sufficient (e.g.[String: Any]
instead ofNSDictionary
)FunctionsErrorTests
FunctionsErrorCode
remains a standalone entity)