Skip to content

pubsub: consider returning non-nil error from NewClient when projectID is invalid #3828

@dmitshur

Description

@dmitshur

Is your feature request related to a problem? Please describe.
This issue is motivated by my experience in golang/go#44948 (comment).

We had code that was publishing to a pub/sub topic that looked roughly like this:

// publishToTopic publishes body to the given topic.
// It returns the ID of the message published.
func publishToTopic(ctx context.Context, topic string, body []byte) (string, error) {
	client, err := pubsub.NewClient(ctx, projectID)
	if err != nil {
		return "", fmt.Errorf("pubsub.NewClient: %v", err)
	}

	t := client.Topic(topic)
	resp := t.Publish(ctx, &pubsub.Message{Data: body})
	id, err := resp.Get(ctx)
	if err != nil {
		return "", fmt.Errorf("topic.Publish: %v", err)
	}
	return id, nil
}

When the value of projectID became the empty string due to an environment misconfiguration, the publishToTopic function failed with the following error message:

topic.Publish: rpc error: code = InvalidArgument desc = You have passed an invalid argument to the service (argument=).

The error message said that I passed an invalid argument, and that the argument I passed was the empty string. With that information, it was quite hard to understand what the problem actually was. I wasn't providing any arguments in the high-level code shown above. (I'm guessing the projectID is one of the internal arguments used when publishing.)

Describe the solution you'd like
As far as I can tell there's no situation where pubsub.NewClient(ctx, "") is a non-error condition.

If that's the case, perhaps the readability of the error in this situation can be improved by having NewClient return something like errors.New("projectID must be a non-empty string") to make the error more readable:

topic.NewClient: projectID must be a non-empty string

Describe alternatives you've considered

  • If the error check cannot be done in NewClient, then perhaps the error message returned by Publish can be improved to be more clear which argument was empty, like "You have passed an invalid value for the projectID argument to the service".

  • If the case where a user accidentally calls pubsub.NewClient(ctx, projectID) with an empty string is deemed rare not to add more complexity to the client code, then another alternative solution is to close this for now and don't take any action unless this problem keeps happening.

Additional context
Issue #1294 may be related.

Metadata

Metadata

Assignees

Labels

api: pubsubIssues related to the Pub/Sub API.type: feature request‘Nice-to-have’ improvement, new feature or different behavior or design.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions