Skip to content

feat(core): Error Handling Revamp #3140

Open
jonathanedey wants to merge 5 commits into
v14from
error-revamp
Open

feat(core): Error Handling Revamp #3140
jonathanedey wants to merge 5 commits into
v14from
error-revamp

Conversation

@jonathanedey
Copy link
Copy Markdown
Collaborator

@jonathanedey jonathanedey commented May 12, 2026

This PR makes changes the usage and handling of Firebase Errors and includes the following changes:

  • Exposes all service-specific error classes, error code types and base FirebaseError class.
  • Exposes the associated uniform error codes types for those respective errors.
  • Decouples previous client-facing error codes from internal SDK details and refactor error handling into dedicated, modular error.ts files, utilizing declaration merging to improve IDE autocomplete and indexing.
  • Exposes ErrorInfo and HttpResponse classes for better error handing.
  • Adds 2 new fields to ErrorInfo FirebaseErrors: cause and HttpResponse.
  • Refactors how FCM HTTP2 session errors are handled making use of the new cause attribute on FirebaseErrors to allow failing of request at the stream level with context on session error cause.
  • Removes internal FirebaseMessagingSessionError and its logic.

* Initial exports and external api changes

* Connect response to error info

* feat: Revamp error handling to include `httpResponse` and `cause` in `FirebaseAuthError` and `FirebaseAppError`, added sample tests.

* more refactoring

* refactor: Update all services to use ErrorInfo format

* refactor: Update unit tests to verify error cause and httpResponse population.

* chore: fix lint

* chore: remove debug tests

* fix: Ran api documentor and fix some export errors

* fix: Removed outdated `__proto__` workaround and updated api doc strings

* chore: Fix lint

* fix: Used a helper function to map `RequestResponse` to `HttpResponse`

* fix: Removed last of the stringified response bodies in response messages

* chore: Fix lint

* chore: Generate apidocs

* fix: Apply revamp changes to pnv

* chore: Added review suggestions
… enable declaration merging (#3127)

* chore(auth): Refactor auth error logic

* chore(installations): Refactor installations and instance id error logic

* chore(fcm): Refactor fcm error logic

* fix(auth): Fix auth error mapping that were not copied correctly

* fix(auth): Fixed `INVALID_SERVICE_ACCOUNT` to map to a valid client code

* chore(pm): Refactor project management error logic

* chore: Fix license year for new files

* chore(rtdb): Refactor rtdb error logic

* chore(fs): Refactor firestore error logic

* chore(app): Refactor app error logic

* chore(security-rules): Refactor security rules error logic

* chore(app-check): Refactor app check error logic

* chore(remote-config): Refactor remote config error logic

* chore(functions): Refactor functions error logic

* chore(extensions): Refactor extensions error logic

* chore(fdc): Refactor data connect error logic

* chore(ml): Refactor ml error logic

* chore(eventarc): Refactor eventarc error logic

* chore(fpnv): Refactor pnv error logic

* fix: address gemini review

* fix: Use Declaration Merging to expose error code constant mapping along side error code type

* fix: Address gemini review

* chore: Remove extra whitespace
* feat(messaging): Improve HTTP/2 session error handling.

* fix: ensure error causes are not lost

* fix: clean up stale tests

* fix: address gemini review

* fix: address more gemini review

* fix: Resolve leftover merge issues
@jonathanedey jonathanedey added release-note release:stage Stage a release candidate labels May 12, 2026
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 SDK's error handling system by introducing a standardized ErrorInfo interface that includes HTTP response data and underlying error causes. Service-specific error classes now extend PrefixedFirebaseError and utilize object-based constructors. The messaging service's HTTP/2 logic was also updated to aggregate session errors into batch responses. Feedback identifies opportunities to improve AggregateError serialization in toJSON(), remove the obsolete FirebaseMessagingSessionError class, and refine error message formatting within the HTTP/2 session handler.

Comment thread src/utils/error.ts
Comment on lines +146 to +150
json.cause = {
name: this.cause.name || 'Error',
message: this.cause.message || String(this.cause),
stack: this.cause.stack
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Since AggregateError is now being used as a cause (specifically in the messaging service), the toJSON() method should be updated to serialize the nested errors array. Without this, critical debugging information about multiple failures will be lost when the error is serialized.

    if (this.cause) {
      json.cause = {
        name: this.cause.name || 'Error',
        message: this.cause.message || String(this.cause),
        stack: this.cause.stack
      };
      if ('errors' in this.cause && Array.isArray((this.cause as any).errors)) {
        (json.cause as any).errors = (this.cause as any).errors.map((e: any) => {
          if (e instanceof Error) {
            return { name: e.name, message: e.message, stack: e.stack };
          }
          return String(e);
        });
      }
    }

Comment thread src/messaging/error.ts
Comment on lines +297 to +318
export class FirebaseMessagingSessionError extends FirebaseMessagingError {
public pendingBatchResponse?: Promise<BatchResponse>;
/**
*
* @param info - The error code info.
* @param message - The error message. This will override the default message if provided.
* @param pendingBatchResponse - BatchResponse for pending messages when session error occured.
*/
constructor(info: ErrorInfo, message?: string, pendingBatchResponse?: Promise<BatchResponse>) {
// Override default message if custom message provided.
super(info, message || info.message);
this.pendingBatchResponse = pendingBatchResponse;
}

/** @returns The object representation of the error. */
public toJSON(): object {
return {
...super.toJSON(),
pendingBatchResponse: this.pendingBatchResponse,
};
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The FirebaseMessagingSessionError class appears to be dead code. The PR description mentions that it and its logic were removed, and the refactoring in src/messaging/messaging.ts confirms that it is no longer thrown. It should be removed from this file as well.

Comment thread src/utils/api-request.ts
Comment on lines +1367 to 1377
this.sessionErrors.push(error);
});

http2Session.on('error', (error: any) => {
this.http2Session.on('error', (error: any) => {
let errorMessage: any;
if (error.name == 'AggregateError' && error.errors) {
errorMessage = `Session error while making requests: ${error.code} - ${error.name}: ` +
`[${error.errors.map((e: any) => e.message).join(', ')}]`
if (error?.name === 'AggregateError' && Array.isArray(error.errors)) {
errorMessage = `Session error while making requests: ${error?.code} - ${error?.name}: ` +
`[${error.errors.map((e: any) => e.message).join(', ')}]`;
} else {
errorMessage = `Session error while making requests: ${error.code} - ${error.message} `
errorMessage = `Session error while making requests: ${error?.code} - ${error?.message} `;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The error message construction in the error event listener can be improved for better readability and type safety. The errorMessage variable should be typed as string, and the logic can be simplified to avoid potential undefined strings and unnecessary trailing spaces.

Suggested change
this.sessionErrors.push(error);
});
http2Session.on('error', (error: any) => {
this.http2Session.on('error', (error: any) => {
let errorMessage: any;
if (error.name == 'AggregateError' && error.errors) {
errorMessage = `Session error while making requests: ${error.code} - ${error.name}: ` +
`[${error.errors.map((e: any) => e.message).join(', ')}]`
if (error?.name === 'AggregateError' && Array.isArray(error.errors)) {
errorMessage = `Session error while making requests: ${error?.code} - ${error?.name}: ` +
`[${error.errors.map((e: any) => e.message).join(', ')}]`;
} else {
errorMessage = `Session error while making requests: ${error.code} - ${error.message} `
errorMessage = `Session error while making requests: ${error?.code} - ${error?.message} `;
}
this.http2Session.on('error', (error: any) => {
const codePart = error?.code ? `${error.code} - ` : '';
let errorMessage: string;
if (error?.name === 'AggregateError' && Array.isArray(error.errors)) {
errorMessage = `Session error while making requests: ${codePart}${error.name}: ` +
`[${error.errors.map((e: any) => e.message).join(', ')}]`;
} else {
errorMessage = `Session error while making requests: ${codePart}${error?.message || 'Unknown error'}`;
}
const appError = new FirebaseAppError({
code: AppErrorCode.NETWORK_ERROR,
message: errorMessage,
cause: error,
});
this.sessionErrors.push(appError);
});

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 release-note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant