Skip to content

feat(core): Error Handling Revamp#3102

Open
jonathanedey wants to merge 18 commits intoerror-revampfrom
je-error-re
Open

feat(core): Error Handling Revamp#3102
jonathanedey wants to merge 18 commits intoerror-revampfrom
je-error-re

Conversation

@jonathanedey
Copy link
Copy Markdown
Collaborator

@jonathanedey jonathanedey commented Mar 30, 2026

This PR makes changes the usage of firebase errors and includes the following changes:

  • Exposes all service-specific error classes, error code types and base FirebaseError class.
  • Exposes the associated error codes types for those errors.
  • Exposes ErrorInfo and HttpResponse classes for better error handing.
  • Adds 2 new fields to ErrorInfo FirebaseErrors: cause and HttpResponse.
  • Converts AuthClientErrorCode, MessagingClientErrorCode, InstallationsClientErrorCode, InstanceIdClientErrorCode from classes to constants.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/auth/auth-api-request.ts Outdated
Comment thread src/auth/auth-api-request.ts Outdated
Comment thread src/auth/auth-api-request.ts Outdated
Comment thread src/auth/auth-api-request.ts Outdated
Comment thread src/app/credential-internal.ts
Comment thread src/app/lifecycle.ts
@jonathanedey jonathanedey changed the base branch from main to error-revamp April 14, 2026 20:17
@jonathanedey jonathanedey marked this pull request as ready for review April 14, 2026 20:17
@jonathanedey jonathanedey added the release:stage Stage a release candidate label Apr 14, 2026
Copy link
Copy Markdown
Member

@lahirumaramba lahirumaramba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks! (added couple comments)

Comment thread src/utils/error.ts Outdated
}

// Warning: (ae-forgotten-export) The symbol "PrefixedFirebaseError" needs to be exported by the entry point index.d.ts
//
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release:stage Stage a release candidate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants