Conversation
…`FirebaseAuthError` and `FirebaseAppError`, added sample tests.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
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.
| ); | ||
| throw new FirebaseAppError({ | ||
| code: AppErrorCodes.INVALID_CREDENTIAL, | ||
| message: `Failed to parse service account json file: ${(error as Error).message}`, |
There was a problem hiding this comment.
Including the low-level error message directly in the main error message string is redundant now that the cause property is available. It's better to keep the main message clean and rely on the cause for the detailed error information. This also avoids potential issues if error is not a standard Error object.
| message: `Failed to parse service account json file: ${(error as Error).message}`, | |
| message: 'Failed to parse service account json file.', |
| throw new FirebaseAppError({ | ||
| code: AppErrorCodes.INVALID_APP_OPTIONS, | ||
| message: `Failed to parse app options file: ${(error as Error).message}`, | ||
| cause: error as Error |
There was a problem hiding this comment.
Similar to the change in credential-internal.ts, it's better to keep the main error message concise and use the cause property to provide the underlying error details.
| cause: error as Error | |
| message: 'Failed to parse app options file.', |
1 participant
No description provided.