googleapis / nodejs-pubsub

Node.js client for Google Cloud Pub/Sub: Ingest event streams from anywhere, at any scale, for simple, reliable, real-time stream analytics.
https://cloud.google.com/pubsub/
Apache License 2.0
516 stars 227 forks source link

Incorrect Handling of Subscription Push Configuration #1915

Closed useCallback closed 2 months ago

useCallback commented 2 months ago

Description:

I encountered an issue while implementing a webhook feature using the Google Pub/Sub SDK for Node.js. Specifically, I was trying to create a push subscription with message unwrapping and without metadata, as outlined in the official documentation. However, despite following the provided instructions, the push subscription was not created with the expected configurations.

Upon further investigation into the source code of the SDK, I discovered an issue in the way push configurations are handled. In the method formatMetadata_ within the subscription.ts file, there is a segment of code that handles the push configuration as follows:

formatted.pushConfig = {
  pushEndpoint: metadata.pushEndpoint,
};
delete formatted.pushEndpoint;

his code snippet only includes the pushEndpoint property in the pushConfig object and removes any other properties. Consequently, when creating a subscription with additional push configuration options, such as message unwrapping (noWrapper), these options are not retained.

To address this issue, I propose a modification to the code to ensure that existing push configuration properties are preserved when setting the pushEndpoint. Here's the updated code snippet:

if (metadata.pushEndpoint) {
  formatted.pushConfig = {
    ...metadata.pushConfig,
    pushEndpoint: metadata.pushEndpoint,
  };
  delete formatted.pushEndpoint;
}

This modification ensures that any existing push configuration options are retained and only the pushEndpoint property is overwritten.

Steps to Reproduce:

Use the Google Pub/Sub SDK for Node.js to create a push subscription. Attempt to include additional push configuration options, such as message unwrapping (noWrapper), in the subscription creation request.

Expected Behavior:

The push subscription should be created with the specified push configuration options, including message unwrapping (noWrapper), as per the documentation.

Actual Behavior:

The push subscription is created without the specified push configuration options, and only the pushEndpoint property is retained.

Proposed Solution:

Modify the handling of push configuration in the formatMetadata_ method to ensure that existing push configuration properties are preserved when setting the pushEndpoint, as described above.

Thank you for your attention to this matter. I believe addressing this issue will improve the usability and reliability of the Google Pub/Sub SDK for Node.js.

Environment details

feywind commented 2 months ago

Thanks for the thorough description and the PR! I think you're right, I'll try to get that merged.