feat(core): Error Handling Revamp#3102
Conversation
…`FirebaseAuthError` and `FirebaseAppError`, added sample tests.
There was a problem hiding this comment.
Code Review
This pull request refactors the error handling across the Firebase Admin SDK by introducing a structured ErrorInfo interface and updating various error classes to accept this object. This allows for better inclusion of httpResponse and cause (underlying error) information, improving debuggability. I have provided feedback regarding missing semicolons in auth-api-request.ts and suggested simplifying error messages by removing redundant low-level error details, as these are now captured by the cause property.
lahirumaramba
left a comment
There was a problem hiding this comment.
LGTM! Thanks! (added couple comments)
| } | ||
|
|
||
| // Warning: (ae-forgotten-export) The symbol "PrefixedFirebaseError" needs to be exported by the entry point index.d.ts | ||
| // |
There was a problem hiding this comment.
Is this something we can address (for both PrefixedFirebaseError and ErrorInfo? I think it is okay to ignore this but I am just double checking :)
There was a problem hiding this comment.
For ErrorInfo it's the same scenario we have for App. Where even though we export it commonly, api documenter wants it to be exported again from other paths where it is used in the definition. So i don't think we have good options other than exposing it from multiple paths.
For PrefixedFirebaseError this is technically internal logic and not exposed at all even though it's a middle step between FirebaseError and the service specific errors. Not sure if there is a way to exclude it from the docs.
This PR makes changes the usage of firebase errors and includes the following changes:
FirebaseErrorclass.ErrorInfoandHttpResponseclasses for better error handing.causeandHttpResponse.AuthClientErrorCode,MessagingClientErrorCode,InstallationsClientErrorCode,InstanceIdClientErrorCodefrom classes to constants.