FirebaseInstallations: CreateInstallation API request error handling #4024
Conversation
maksymmalyhin
commented
Oct 8, 2019
•
maksymmalyhin
commented
- avoid sending CreateInstallation API requests when Firebase configuration is invalid (see go/fis-ios-error-handling for details).
This reverts commit a8e0641.
| NSString *APIKey = self.APIService.APIKey; | ||
| NSString *projectID = self.APIService.projectID; | ||
| return (parameters.APIKey == APIKey || [parameters.APIKey isEqual:APIKey]) && | ||
| (parameters.projectID == projectID || [parameters.projectID isEqual:projectID]); |
There was a problem hiding this comment.
parameters.APIKey == APIKey
Is this for comparing the address to be the same as well?
There was a problem hiding this comment.
I use parameters.APIKey == APIKey to cover the case when they are both nil. [parameters.APIKey isEqual:APIKey] returns NO in this case. It is the easiest way to cover all equality cases I came up with, but I am open for suggestions.
There was a problem hiding this comment.
nice! I didn't know that.
There was a problem hiding this comment.
yeah, I didn't pay attention to it until the integration tests failures. Basically if e.g. parameters.APIKey == nil then any message to it will return nil which is casted to NO.
|
|
||
| // Save the error and then fail with the API error. | ||
| return | ||
| [self.installationsStore saveInstallation:failedInstallation].then(^NSError *(id result) { |
There was a problem hiding this comment.
If registration returns with error, why do we still save such installation item?
There was a problem hiding this comment.
I've added a design doc link explaining the details (sorry I should've added it before). TL;DR: we would like to store the registration parameters and the error received, so we can return the error without sending the API request. We will send the API request again only if the parameters changed.
There was a problem hiding this comment.
what if the error is because network is not available?
There was a problem hiding this comment.
In this case [self doesRegistrationErrorRequireConfigChange:error] at line # 269 will return NO so the error will not be saved and the installation status will stay "unregistered".
…simple log message. (#4120) * Revert "FIS: stored registration error timeout (#4032)" This reverts commit 37747db. * Revert "FirebaseInstallations: CreateInstallation API request error handling (#4024)" This reverts commit 27384c0. * Complex unrecoverable error handling reverted and replaced by a simple log message.
