[PLUGIN-1868] Error Management for Google Publisher #1524

AnkitCLI · 2025-02-24T16:00:16Z

Description
Error Management for Google Publisher Sink plugin exceptions

https://cdap.atlassian.net/browse/PLUGIN-1868

Code change
Modified GooglePublisher.java
Added GooglePublisherErrorDetailsProvider.java
Modified GCPUtils.java
Modified PubSubOutputFormat.java

Tests
Test Case - tested with valid and invalid scenarios

1. Success

2. Permission denied to create topic

Logs

3. Failed to publish records

BQ plugin after changes

GCS move plugin after changes

itsankit-google · 2025-02-28T06:04:15Z

src/main/java/io/cdap/plugin/gcp/publisher/GooglePublisher.java

+            String error = String.format(
+              "Could not auto-create topic '%s' in project '%s'. "
+                + "Please ensure it is created before running the pipeline, "
+                + "or ensure that the service account has permission to create the topic.",
+              config.topic, projectId);
+            throw ErrorUtils.getProgramFailureException(new ErrorCategory(ErrorCategory.ErrorCategoryEnum.PLUGIN),
+                                                        error, error, ErrorType.USER, false, e1);


ApiException should have much more details, like statusCode which can help in computing error type. Dependency should also be true.

Updated with the status code and dependency

itsankit-google · 2025-02-28T06:05:29Z

src/main/java/io/cdap/plugin/gcp/publisher/PubSubOutputFormat.java

+        String error = String.format("Error publishing records to PubSub: %s", e.getMessage());
+        throw ErrorUtils.getProgramFailureException(
+          new ErrorCategory(ErrorCategory.ErrorCategoryEnum.PLUGIN),
+          error, error, ErrorType.USER, false, e);


can ExecutionException contain GoogleJsonResponseException or ApiException? Did we check the stacktrace?

Updated and SS attached in the description.

itsankit-google · 2025-03-03T05:59:08Z

src/main/java/io/cdap/plugin/gcp/publisher/GooglePublisher.java

+                + "Status code: %d.",
+              config.topic, projectId, statusCode);
+            throw ErrorUtils.getProgramFailureException(new ErrorCategory(ErrorCategory.ErrorCategoryEnum.PLUGIN),
+                                                        error, error, ErrorType.USER, true, e1);


Add errorCode, errorType and supportDocUrl to the exception.

This comment applies to the whole PR.

itsankit-google · 2025-03-03T14:27:09Z

src/main/java/io/cdap/plugin/gcp/publisher/PubSubOutputFormat.java

+        String errorMessage = String.format("Failed to publish %s records: %s. For more details, see %s",
+        failures.get(), error.get(), GCPUtils.PUBSUB_SUPPORTED_DOC_URL);
+        throw ErrorUtils.getProgramFailureException(new ErrorCategory(ErrorCategory.ErrorCategoryEnum.PLUGIN),
+        errorMessage, errorMessage, ErrorType.USER, false, null, null, GCPUtils.PUBSUB_SUPPORTED_DOC_URL, null);


Do not add support doc when error is not from external client.

Why error type USER?

Doc removed, changed user to unknown

itsankit-google · 2025-03-03T14:28:02Z

src/main/java/io/cdap/plugin/gcp/publisher/PubSubOutputFormat.java

+        throw GCPErrorDetailsProviderUtil.getHttpResponseExceptionDetailsFromChain(e, errorReason, ErrorType.USER,
+        true, GCPUtils.PUBSUB_SUPPORTED_DOC_URL);


can ExecutionException contain GoogleJsonResponseException or ApiException? Did we check the stacktrace?

Why error type USER?

I suggest we make this unknown, error type is correctly detected when using getHttpResponseExceptionDetailsFromChain using the http error codes, else it should be unknown.

changed to UNKNOWN

itsankit-google

why does the 3rd screenshot in PR description does not have errorCode in the response?

Didn't the stacktrace had GoogleJsonResponseException? If not then we should also handle ApiException in GcpErrorDetailsProviderUtil.

AnkitCLI · 2025-03-04T17:55:42Z

why does the 3rd screenshot in PR description does not have errorCode in the response?

Didn't the stacktrace had GoogleJsonResponseException? If not then we should also handle ApiException in GcpErrorDetailsProviderUtil.

Handled APiException in GcpErrorDetailsProviderUtil. Attached updated SS in the description .

itsankit-google · 2025-03-04T18:08:40Z

why does the 3rd screenshot in PR description does not have errorCode in the response?
Didn't the stacktrace had GoogleJsonResponseException? If not then we should also handle ApiException in GcpErrorDetailsProviderUtil.

Handled APiException in GcpErrorDetailsProviderUtil. Attached updated SS in the description .

Did we check the stacktrace? This change will affect all GCP plugins. Can you please add the stacktrace and pipeline logs?

AnkitCLI · 2025-03-05T07:18:10Z

why does the 3rd screenshot in PR description does not have errorCode in the response?
Didn't the stacktrace had GoogleJsonResponseException? If not then we should also handle ApiException in GcpErrorDetailsProviderUtil.

Handled APiException in GcpErrorDetailsProviderUtil. Attached updated SS in the description .

Did we check the stacktrace? This change will affect all GCP plugins. Can you please add the stacktrace and pipeline logs?

Yes we checked the stacktrace

src/main/java/io/cdap/plugin/gcp/common/GCPErrorDetailsProviderUtil.java

itsankit-google · 2025-03-06T13:37:01Z

src/main/java/io/cdap/plugin/gcp/publisher/GooglePublisher.java

+                + "For more details, see %s",
+              config.topic, projectId, GCPUtils.PUBSUB_SUPPORTED_DOC_URL);
+            throw ErrorUtils.getProgramFailureException(new ErrorCategory(ErrorCategory.ErrorCategoryEnum.PLUGIN),
+            error, error, ErrorType.USER, true, ErrorCodeType.HTTP, String.valueOf(statusCode),


ErrorType.UNKNOWN by default

src/main/java/io/cdap/plugin/gcp/common/GCPErrorDetailsProviderUtil.java

itsankit-google

Can you please also re-test somecases of GCS & BQ plugin to verify they are working as expected after these changes?

src/main/java/io/cdap/plugin/gcp/common/GCPErrorDetailsProviderUtil.java

itsankit-google · 2025-03-07T06:29:31Z

src/main/java/io/cdap/plugin/gcp/common/GCPErrorDetailsProvider.java

      if (t instanceof IllegalArgumentException) {
        return getProgramFailureException((IllegalArgumentException) t, errorContext);
      }
      if (t instanceof IllegalStateException) {
        return getProgramFailureException((IllegalStateException) t, errorContext);
      }


these two checks can also go in later loop.

AnkitCLI · 2025-03-07T06:50:18Z

Can you please also re-test somecases of GCS & BQ plugin to verify they are working as expected after these changes?

checkedn with GCS and BQ both. Working fine.

AnkitCLI added the build Trigger unit test build label Feb 24, 2025

AnkitCLI marked this pull request as ready for review February 24, 2025 16:13

AnkitCLI requested a review from psainics February 24, 2025 16:18

psainics marked this pull request as draft February 25, 2025 06:54

AnkitCLI force-pushed the em/googlePublisher branch 3 times, most recently from 1a3d5fe to 764c443 Compare February 27, 2025 06:12

AnkitCLI marked this pull request as ready for review February 27, 2025 06:12

psainics assigned AnkitCLI Feb 27, 2025

psainics requested a review from itsankit-google February 27, 2025 07:11

psainics changed the title ~~Error Management for Google Publisher~~ [PLUGIN-1868] Error Management for Google Publisher Feb 27, 2025

itsankit-google reviewed Feb 28, 2025

View reviewed changes

AnkitCLI force-pushed the em/googlePublisher branch from 764c443 to 0837503 Compare March 3, 2025 05:23

itsankit-google reviewed Mar 3, 2025

View reviewed changes

AnkitCLI force-pushed the em/googlePublisher branch from 0837503 to d12ef2f Compare March 3, 2025 08:15

psainics requested a review from itsankit-google March 3, 2025 14:18

itsankit-google reviewed Mar 3, 2025

View reviewed changes

AnkitCLI force-pushed the em/googlePublisher branch from d12ef2f to ef5d48f Compare March 3, 2025 15:57

AnkitCLI requested a review from itsankit-google March 4, 2025 05:46

itsankit-google reviewed Mar 4, 2025

View reviewed changes

AnkitCLI force-pushed the em/googlePublisher branch from ef5d48f to 5de6a5d Compare March 4, 2025 17:51

AnkitCLI requested a review from itsankit-google March 4, 2025 18:02

AnkitCLI force-pushed the em/googlePublisher branch 3 times, most recently from ca2a4cf to da97000 Compare March 5, 2025 07:16

AnkitCLI requested review from itsankit-google and removed request for itsankit-google March 5, 2025 07:22