Skip to content
This repository has been archived by the owner on Jun 19, 2022. It is now read-only.

Commit

Permalink
Fix wrong status message by changing %w to %v. (#1675)
Browse files Browse the repository at this point in the history
Experiment shows that `%w` does not invoke the `Error() string` method
that the error type provides for proper struct formatting. When using
it in status message or logger, we will get sth like:
```
- lastTransitionTime: "2020-09-09T13:23:24Z"
    message: 'Failed to verify Pub/Sub topic exists: %!w(*status.Error=&{0xc0024ffe60})'
    reason: TopicVerificationFailed
```

After the fix, it will show as:

```
- lastTransitionTime: "2020-09-09T14:13:41Z"
    message: |-
      Failed to verify Pub/Sub topic exists: rpc error: code = Unauthenticated desc = transport: oauth2: cannot fetch token: 400 Bad Request
      Response: {"error":"invalid_grant","error_description":"Invalid JWT Signature."}
    reason: TopicVerificationFailed
```
  • Loading branch information
Zhongduo Lin (Jimmy) authored Sep 9, 2020
1 parent f84ca97 commit 3ed2eff
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 26 deletions.
2 changes: 1 addition & 1 deletion pkg/pubsub/adapter/adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func (a *Adapter) Stop() {
func (a *Adapter) receive(ctx context.Context, msg *pubsub.Message) {
event, err := a.converter.Convert(ctx, msg, a.args.ConverterType)
if err != nil {
a.logger.Debug("Failed to convert received message to an event, check the msg format: %w", zap.Error(err))
a.logger.Debug("Failed to convert received message to an event, check the msg format: %v", zap.Error(err))
// Ack the message so it won't be retried, we consider all errors to be non-retryable.
msg.Ack()
return
Expand Down
16 changes: 8 additions & 8 deletions pkg/reconciler/broker/broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ func (r *Reconciler) reconcileDecouplingTopicAndSubscription(ctx context.Context
projectID, err := utils.ProjectID(r.projectID, metadataClient.NewDefaultMetadataClient())
if err != nil {
logger.Error("Failed to find project id", zap.Error(err))
b.Status.MarkTopicUnknown("ProjectIdNotFound", "Failed to find project id: %w", err)
b.Status.MarkSubscriptionUnknown("ProjectIdNotFound", "Failed to find project id: %w", err)
b.Status.MarkTopicUnknown("ProjectIdNotFound", "Failed to find project id: %v", err)
b.Status.MarkSubscriptionUnknown("ProjectIdNotFound", "Failed to find project id: %v", err)
return err
}
// Set the projectID in the status.
Expand All @@ -126,8 +126,8 @@ func (r *Reconciler) reconcileDecouplingTopicAndSubscription(ctx context.Context
client, err = pubsub.NewClient(ctx, projectID)
if err != nil {
logger.Error("Failed to create Pub/Sub client", zap.Error(err))
b.Status.MarkTopicUnknown("PubSubClientCreationFailed", "Failed to create Pub/Sub client: %w", err)
b.Status.MarkSubscriptionUnknown("PubSubClientCreationFailed", "Failed to create Pub/Sub client: %w", err)
b.Status.MarkTopicUnknown("PubSubClientCreationFailed", "Failed to create Pub/Sub client: %v", err)
b.Status.MarkSubscriptionUnknown("PubSubClientCreationFailed", "Failed to create Pub/Sub client: %v", err)
return err
}
defer client.Close()
Expand Down Expand Up @@ -181,8 +181,8 @@ func (r *Reconciler) deleteDecouplingTopicAndSubscription(ctx context.Context, b
projectID, err := utils.ProjectID(r.projectID, metadataClient.NewDefaultMetadataClient())
if err != nil {
logger.Error("Failed to find project id", zap.Error(err))
b.Status.MarkTopicUnknown("FinalizeTopicProjectIdNotFound", "Failed to find project id: %w", err)
b.Status.MarkSubscriptionUnknown("FinalizeSubscriptionProjectIdNotFound", "Failed to find project id: %w", err)
b.Status.MarkTopicUnknown("FinalizeTopicProjectIdNotFound", "Failed to find project id: %v", err)
b.Status.MarkSubscriptionUnknown("FinalizeSubscriptionProjectIdNotFound", "Failed to find project id: %v", err)
return err
}

Expand All @@ -191,8 +191,8 @@ func (r *Reconciler) deleteDecouplingTopicAndSubscription(ctx context.Context, b
client, err := pubsub.NewClient(ctx, projectID)
if err != nil {
logger.Error("Failed to create Pub/Sub client", zap.Error(err))
b.Status.MarkTopicUnknown("FinalizeTopicPubSubClientCreationFailed", "Failed to create Pub/Sub client: %w", err)
b.Status.MarkSubscriptionUnknown("FinalizeSubscriptionPubSubClientCreationFailed", "Failed to create Pub/Sub client: %w", err)
b.Status.MarkTopicUnknown("FinalizeTopicPubSubClientCreationFailed", "Failed to create Pub/Sub client: %v", err)
b.Status.MarkSubscriptionUnknown("FinalizeSubscriptionPubSubClientCreationFailed", "Failed to create Pub/Sub client: %v", err)
return err
}
defer client.Close()
Expand Down
16 changes: 8 additions & 8 deletions pkg/reconciler/trigger/trigger.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,8 @@ func (r *Reconciler) reconcileRetryTopicAndSubscription(ctx context.Context, tri
projectID, err := utils.ProjectID(r.projectID, metadataClient.NewDefaultMetadataClient())
if err != nil {
logger.Error("Failed to find project id", zap.Error(err))
trig.Status.MarkTopicUnknown("ProjectIdNotFound", "Failed to find project id: %w", err)
trig.Status.MarkSubscriptionUnknown("ProjectIdNotFound", "Failed to find project id: %w", err)
trig.Status.MarkTopicUnknown("ProjectIdNotFound", "Failed to find project id: %v", err)
trig.Status.MarkSubscriptionUnknown("ProjectIdNotFound", "Failed to find project id: %v", err)
return err
}
// Set the projectID in the status.
Expand All @@ -204,8 +204,8 @@ func (r *Reconciler) reconcileRetryTopicAndSubscription(ctx context.Context, tri
client, err := pubsub.NewClient(ctx, projectID)
if err != nil {
logger.Error("Failed to create Pub/Sub client", zap.Error(err))
trig.Status.MarkTopicUnknown("PubSubClientCreationFailed", "Failed to create Pub/Sub client: %w", err)
trig.Status.MarkSubscriptionUnknown("PubSubClientCreationFailed", "Failed to create Pub/Sub client: %w", err)
trig.Status.MarkTopicUnknown("PubSubClientCreationFailed", "Failed to create Pub/Sub client: %v", err)
trig.Status.MarkSubscriptionUnknown("PubSubClientCreationFailed", "Failed to create Pub/Sub client: %v", err)
return err
}
defer client.Close()
Expand Down Expand Up @@ -298,8 +298,8 @@ func (r *Reconciler) deleteRetryTopicAndSubscription(ctx context.Context, trig *
projectID, err := utils.ProjectID(r.projectID, metadataClient.NewDefaultMetadataClient())
if err != nil {
logger.Error("Failed to find project id", zap.Error(err))
trig.Status.MarkTopicUnknown("FinalizeTopicProjectIdNotFound", "Failed to find project id: %w", err)
trig.Status.MarkSubscriptionUnknown("FinalizeSubscriptionProjectIdNotFound", "Failed to find project id: %w", err)
trig.Status.MarkTopicUnknown("FinalizeTopicProjectIdNotFound", "Failed to find project id: %v", err)
trig.Status.MarkSubscriptionUnknown("FinalizeSubscriptionProjectIdNotFound", "Failed to find project id: %v", err)
return err
}

Expand All @@ -308,8 +308,8 @@ func (r *Reconciler) deleteRetryTopicAndSubscription(ctx context.Context, trig *
client, err := pubsub.NewClient(ctx, projectID)
if err != nil {
logger.Error("Failed to create Pub/Sub client", zap.Error(err))
trig.Status.MarkTopicUnknown("FinalizeTopicPubSubClientCreationFailed", "Failed to create Pub/Sub client: %w", err)
trig.Status.MarkSubscriptionUnknown("FinalizeSubscriptionPubSubClientCreationFailed", "Failed to create Pub/Sub client: %w", err)
trig.Status.MarkTopicUnknown("FinalizeTopicPubSubClientCreationFailed", "Failed to create Pub/Sub client: %v", err)
trig.Status.MarkSubscriptionUnknown("FinalizeSubscriptionPubSubClientCreationFailed", "Failed to create Pub/Sub client: %v", err)
return err
}
defer client.Close()
Expand Down
10 changes: 5 additions & 5 deletions pkg/reconciler/utils/pubsub/subscription.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (r *Reconciler) ReconcileSubscription(ctx context.Context, id string, subCo
subExists, err := sub.Exists(ctx)
if err != nil {
logger.Error("Failed to verify Pub/Sub subscription exists", zap.Error(err))
updater.MarkSubscriptionUnknown("SubscriptionVerificationFailed", "Failed to verify Pub/Sub subscription exists: %w", err)
updater.MarkSubscriptionUnknown("SubscriptionVerificationFailed", "Failed to verify Pub/Sub subscription exists: %v", err)
return nil, err
}

Expand All @@ -50,7 +50,7 @@ func (r *Reconciler) ReconcileSubscription(ctx context.Context, id string, subCo
config, err := sub.Config(ctx)
if err != nil {
logger.Error("Failed to get Pub/Sub subscription Config", zap.Error(err))
updater.MarkSubscriptionUnknown("SubscriptionConfigUnknown", "Failed to get Pub/Sub subscription Config: %w", err)
updater.MarkSubscriptionUnknown("SubscriptionConfigUnknown", "Failed to get Pub/Sub subscription Config: %v", err)
return nil, err
}
if config.Topic != nil && config.Topic.String() == deletedTopic {
Expand Down Expand Up @@ -79,12 +79,12 @@ func (r *Reconciler) DeleteSubscription(ctx context.Context, id string, obj runt
exists, err := sub.Exists(ctx)
if err != nil {
logger.Error("Failed to verify Pub/Sub subscription exists", zap.Error(err))
updater.MarkSubscriptionUnknown("FinalizeSubscriptionVerificationFailed", "failed to verify Pub/Sub subscription exists: %w", err)
updater.MarkSubscriptionUnknown("FinalizeSubscriptionVerificationFailed", "failed to verify Pub/Sub subscription exists: %v", err)
return err
}
if exists {
if err = r.deleteSubscription(ctx, sub, obj); err != nil {
updater.MarkSubscriptionUnknown("FinalizeSubscriptionDeletionFailed", "failed to delete Pub/Sub subscription: %w", err)
updater.MarkSubscriptionUnknown("FinalizeSubscriptionDeletionFailed", "failed to delete Pub/Sub subscription: %v", err)
return err
}
}
Expand All @@ -108,7 +108,7 @@ func (r *Reconciler) createSubscription(ctx context.Context, id string, subConfi
sub, err := r.client.CreateSubscription(ctx, id, subConfig)
if err != nil {
logger.Error("Failed to create subscription", zap.Error(err))
updater.MarkSubscriptionFailed("SubscriptionCreationFailed", "Subscription creation failed: %w", err)
updater.MarkSubscriptionFailed("SubscriptionCreationFailed", "Subscription creation failed: %v", err)
return nil, err
}
logger.Info("Created PubSub subscription", zap.String("name", sub.ID()))
Expand Down
8 changes: 4 additions & 4 deletions pkg/reconciler/utils/pubsub/topic.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func (r *Reconciler) ReconcileTopic(ctx context.Context, id string, topicConfig
exists, err := topic.Exists(ctx)
if err != nil {
logger.Error("Failed to verify Pub/Sub topic exists", zap.Error(err))
updater.MarkTopicUnknown("TopicVerificationFailed", "Failed to verify Pub/Sub topic exists: %w", err)
updater.MarkTopicUnknown("TopicVerificationFailed", "Failed to verify Pub/Sub topic exists: %v", err)
return nil, err
}
if exists {
Expand All @@ -52,7 +52,7 @@ func (r *Reconciler) ReconcileTopic(ctx context.Context, id string, topicConfig
topic, err = r.client.CreateTopicWithConfig(ctx, id, topicConfig)
if err != nil {
logger.Error("Failed to create Pub/Sub topic", zap.Error(err))
updater.MarkTopicFailed("TopicCreationFailed", "Topic creation failed: %w", err)
updater.MarkTopicFailed("TopicCreationFailed", "Topic creation failed: %v", err)
return nil, err
}
logger.Info("Created PubSub topic", zap.String("name", topic.ID()))
Expand All @@ -71,13 +71,13 @@ func (r *Reconciler) DeleteTopic(ctx context.Context, id string, obj runtime.Obj
exists, err := topic.Exists(ctx)
if err != nil {
logger.Error("Failed to verify Pub/Sub topic exists", zap.Error(err))
updater.MarkTopicUnknown("FinalizeTopicVerificationFailed", "failed to verify Pub/Sub topic exists: %w", err)
updater.MarkTopicUnknown("FinalizeTopicVerificationFailed", "failed to verify Pub/Sub topic exists: %v", err)
return err
}
if exists {
if err := topic.Delete(ctx); err != nil {
logger.Error("Failed to delete Pub/Sub topic", zap.Error(err))
updater.MarkTopicUnknown("FinalizeTopicDeletionFailed", "failed to delete Pub/Sub topic: %w", err)
updater.MarkTopicUnknown("FinalizeTopicDeletionFailed", "failed to delete Pub/Sub topic: %v", err)
return err
}
logger.Info("Deleted PubSub topic", zap.String("name", topic.ID()))
Expand Down

0 comments on commit 3ed2eff

Please sign in to comment.