diff --git a/api/api/projects_api.go b/api/api/projects_api.go index a2998a0c..1a0ce1cb 100644 --- a/api/api/projects_api.go +++ b/api/api/projects_api.go @@ -9,6 +9,7 @@ import ( "github.com/caraml-dev/mlp/api/log" "github.com/caraml-dev/mlp/api/models" apperror "github.com/caraml-dev/mlp/api/pkg/errors" + "github.com/caraml-dev/mlp/api/pkg/webhooks" ) type ProjectsController struct { @@ -51,7 +52,19 @@ func (c *ProjectsController) CreateProject(r *http.Request, vars map[string]stri user := vars["user"] project.Administrators = addRequester(user, project.Administrators) project, err = c.ProjectsService.CreateProject(r.Context(), project) + var webhookError *webhooks.WebhookError if err != nil { + // NOTE: Here we are checking if the error is a WebhookError + // This is to improve the error message shared with the user, + // since the current logic creates the Project first before firing + // the ProjectCreatedEvent webhook. + if errors.As(err, &webhookError) { + errMsg := fmt.Errorf(`Project %s was created, + but not all webhooks were correctly invoked. + Some additional resources may not have been created successfully`, project.Name) + log.Errorf("%s, %s", errMsg, err) + return FromError(errMsg) + } log.Errorf("error creating project %s: %s", project.Name, err) return FromError(err) } diff --git a/api/pkg/webhooks/client.go b/api/pkg/webhooks/client.go index 35e0d454..e4627947 100644 --- a/api/pkg/webhooks/client.go +++ b/api/pkg/webhooks/client.go @@ -103,14 +103,14 @@ func (g *simpleWebhookClient) Invoke(ctx context.Context, payload []byte) ([]byt } // check http status code if resp.StatusCode != http.StatusOK { - return fmt.Errorf("response status code %d not 200", resp.StatusCode) + return fmt.Errorf("response status code %d not 200, err: %s", resp.StatusCode, content) } return nil - }, retry.Attempts(uint(g.NumRetries)), retry.Context(ctx), + }, retry.Attempts(uint(g.NumRetries)), retry.Context(ctx), retry.LastErrorOnly(true), ) if err != nil { - return nil, err + return nil, NewWebhookError(err) } return content, nil } @@ -152,4 +152,7 @@ func setDefaults(webhookConfig *WebhookConfig) { def := 10 webhookConfig.Timeout = &def } + if webhookConfig.NumRetries == 0 { + webhookConfig.NumRetries = 3 + } } diff --git a/api/pkg/webhooks/errors.go b/api/pkg/webhooks/errors.go new file mode 100644 index 00000000..f8287f0c --- /dev/null +++ b/api/pkg/webhooks/errors.go @@ -0,0 +1,29 @@ +package webhooks + +import "fmt" + +type WebhookError struct { + msg string + err error +} + +func NewWebhookError(err error) *WebhookError { + return &WebhookError{ + msg: "error invoking webhook", + err: err, + } +} + +// Implement errors.Error method +func (e *WebhookError) Error() string { + return fmt.Sprintf("%s: %v", e.msg, e.err) +} + +func (e *WebhookError) Unwrap() error { + return e.err +} + +func (e *WebhookError) Is(target error) bool { + _, ok := target.(*WebhookError) + return ok +} diff --git a/api/service/projects_service.go b/api/service/projects_service.go index 5f366155..df8fc586 100644 --- a/api/service/projects_service.go +++ b/api/service/projects_service.go @@ -17,6 +17,7 @@ import ( "github.com/caraml-dev/mlp/api/repository" "github.com/caraml-dev/mlp/api/config" + "github.com/caraml-dev/mlp/api/log" "github.com/caraml-dev/mlp/api/models" "github.com/caraml-dev/mlp/api/pkg/authz/enforcer" "github.com/caraml-dev/mlp/api/pkg/webhooks" @@ -107,18 +108,21 @@ func (service *projectsService) CreateProject(ctx context.Context, project *mode // Expects webhook output to be a project object var tmpproject models.Project if err := json.Unmarshal(p, &tmpproject); err != nil { - return err + return webhooks.NewWebhookError(err) } project, err = service.save(&tmpproject) if err != nil { - return err + return webhooks.NewWebhookError(err) } return nil - }, webhooks.NoOpErrorHandler) + }, func(err error) error { + // Print error and return + log.Errorf("error calling webhook - %s, err: %s", ProjectCreatedEvent, err.Error()) + return err + }, + ) if err != nil { - return project, - fmt.Errorf("error while invoking %s webhooks or on success callback function, err: %s", - ProjectCreatedEvent, err.Error()) + return project, err } return project, nil } @@ -165,11 +169,14 @@ func (service *projectsService) UpdateProject(ctx context.Context, project *mode return err } return nil - }, webhooks.NoOpErrorHandler) + }, func(err error) error { + // Print error and return + log.Errorf("error calling webhook - %s, err: %s", ProjectCreatedEvent, err.Error()) + return err + }, + ) if err != nil { - return project, nil, - fmt.Errorf("error while invoking %s webhooks or on success callback function, err: %s", - ProjectUpdatedEvent, err.Error()) + return project, nil, err } } else { project, err = service.save(project)