Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error codes in TopicManagementResponse are incomplete, causing loss of important debug information #1072

Open
JosRoseboom opened this issue Feb 5, 2025 · 2 comments

Comments

@JosRoseboom
Copy link

[REQUIRED] Step 2: Describe your environment

  • Operating System version: MacOs 15.1.1 and docker from arm64v8/amazoncorretto:21-al2023-headless
  • Firebase SDK version: 9.4.3
  • Library version: firebase-admin-java
  • Firebase Product: firebase-admin

[REQUIRED] Step 3: Describe the problem

TopicManagementResponse has currently 4 error codes:

  private static final Map<String, String> ERROR_CODES = ImmutableMap.<String, String>builder()
      .put("INVALID_ARGUMENT", "invalid-argument")
      .put("NOT_FOUND", "registration-token-not-registered")
      .put("INTERNAL", "internal-error")
      .put("TOO_MANY_TOPICS", "too-many-topics")

Looking at https://developers.google.com/instance-id/reference/server , I see that it is currently also possible that RESOURCE_EXHAUSTED is returned. We log the response if there are any errors and currently see a lot of:

[Error{index=0, reason=unknown-error}]

This is not helpful, while it could be caused by RESOURCE_EXHAUSTED, which would be really good to know.

The second problem is that the error string comes from :

 private Error(int index, String reason) {
      this.index = index;
this.reason = ERROR_CODES.containsKey(reason)
        ? ERROR_CODES.get(reason) : UNKNOWN_ERROR;

If the reason is not in the predefined ERROR_CODES it would be more helpful to use the reason String that came in as an parameter than UNKNOWN_ERROR. UNKNOWN_ERROR could be used for null or empty Strings.

Steps to reproduce:

Cause a RESOURCE_EXHAUSTED and see that the error description is unknown-error

Solution

I went ahead and patched this for our own use. I will create a PR containing a commit that adds RESOURCE_EXHAUSTED and a commit that uses the parameter reason as fallback in case of a non empty string.

@google-oss-bot
Copy link

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

@JosRoseboom
Copy link
Author

I created a pull request for this issue: #1073

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

No branches or pull requests

2 participants