Static services errors

[NicolasMahe]

https://github.com/mesg-foundation/core/blob/bd84ad1d54eadfb8aa2bd79ac0c7c68f775f9c03/service/error.go#L20-L26
https://github.com/mesg-foundation/core/blob/bd84ad1d54eadfb8aa2bd79ac0c7c68f775f9c03/service/error.go#L45-L51
https://github.com/mesg-foundation/core/blob/bd84ad1d54eadfb8aa2bd79ac0c7c68f775f9c03/service/error.go#L70-L76

I really don't like to call validate function in the Error() function.
I think the errors should already contain everything needed to display the full error as a string. @mesg-foundation/core what do you thinks guys? Moreover, the app already call the validate functions in order to know if the data is valid.

[ilgooz]

we definitely need to refactor validation error types. related with the validation methods described here: #337.

[ilgooz]

related with #347, #348, #349, #337.

this can be a solution:

type InvalidOutputDataError struct {
	Key      string
	Warnings []*ParameterWarning
}

func (e InvalidOutputDataError) Error() string {
	s := fmt.Sprintf("Outputs of task %q are invalid", e.Key)
	for _, warning := range e.Warnings {
		s = fmt.Sprintf("%s. %s" + warning())
	}
	return s
}

or lets be more generic:

const (
	InvalidTaskInput invalidData = iota + 1
	InvalidTaskOutput
	InvalidEventData
)

type invalidData int

type InvalidDataError struct {
	Type     invalidData
	Key      string
	Warnings []*ParameterWarning
}

func (e InvalidDataError) Error() string {
	var w string
	switch e.Type {
	case InvalidTaskInput:
		w = "Inputs of task"
	case InvalidTaskOutput:
		w = "Outputs of task"
	case InvalidEventData:
		w = "Data of event"
	}
	s := fmt.Sprintf("%s %q are invalid", w, e.Key)
	for _, warning := range e.Warnings {
		s = fmt.Sprintf("%s. %s" + warning)
	}
	return s
}

and for the not found errors:

const (
	EventNotFound notFound = iota + 1
	TaskNotFound
	TaskInputNotFound
	TaskOutputNotFound
)

type notFound int

type NotFoundError struct {
	Type      notFound
	ServiceID string
	Key       string
}

func (e NotFoundError) Error() string {
	var w string
	switch e.Type {
	case EventNotFound:
		w = "Event"
	case TaskNotFound:
		w = "Task"
	case TaskInputNotFound:
		w = "Input"
	case TaskOutputNotFound:
		w = "Output"
	}
	return fmt.Sprintf("%s %q not found in service %q", w, e.Key, e.ServiceID)
}

finally they can be used like below with the new APIs of service package described at #337:

manager := service.NewManager()
service, _ := manager.GetService("v1_x")

event, err := service.GetEvent("request")
// err can be a `NotFound` error or nil
// NotFound can be type asserted like: `eventNotFound := err.(NotFound)``
// access to Type, Key or ServiceID like `eventNotFound.Key` when needed

err := event.ValidateParameters()
// err can be a `InvalidDataError` error or nil
// InvalidDataError can be type asserted like: `invalidEventData := err.(InvalidDataError)`
// access to Type, Key or Warnings like `invalidEventData.Warnings` when needed

[NicolasMahe]

I'm ok to put directly the warnings in the errors.
But not for the generic version: it removes too much context.

[NicolasMahe]

https://github.com/mesg-foundation/core/blob/39ad2a2ea8e7228840f55990bb16369af26c8781/service/error.go#L65-L69

( 2 differents keys)

Versus

type NotFoundError struct {
	Type      notFound
	ServiceID string
	Key       string
}

[ilgooz]

no worries, we have a support for that in the updated version. do you like following approach?

package service

import "fmt"

const (
	EventNotFound notFound = iota + 1
	TaskNotFound
	TaskInputNotFound
	TaskOutputNotFound
)

type notFound int

type NotFoundError struct {
	Type          notFound
	ServiceName   string
	EventKey      string
	TaskKey       string
	TaskInputKey  string
	TaskOutputKey string
}

func (e NotFoundError) Error() string {
	p := fmt.Sprintf

	switch e.Type {
	case EventNotFound:
		return p("Event %q not found in service %q", e.EventKey, e.ServiceName)

	case TaskNotFound:
		return p("Task %q not found in service %q", e.TaskKey, e.ServiceName)

	case TaskInputNotFound:
		return p("Input %q of task %q not found in service %q", e.TaskInputKey, e.TaskKey, e.ServiceName)

	case TaskOutputNotFound:
		return p("Output %q of task %q not found in service %q", e.TaskOutputKey, e.TaskKey, e.ServiceName)
	}

	return ""
}

const (
	InvalidTaskInput invalidData = iota + 1
	InvalidTaskOutput
	InvalidEventData
)

type invalidData int

type InvalidDataError struct {
	Type          invalidData
	Warnings      []*ParameterWarning
	EventKey      string
	TaskKey       string
	TaskOutputKey string
}

func (e InvalidDataError) Error() string {
	var (
		p = fmt.Sprintf
		s string
	)

	switch e.Type {
	case InvalidTaskInput:
		s = p("Inputs of task %q are invalid", e.TaskKey)

	case InvalidTaskOutput:
		s = p("Outputs %q of task %q are invalid", e.TaskOutputKey, e.TaskKey)

	case InvalidEventData:
		s = p("Data of event %q is invalid", e.EventKey)
	}

	for _, warning := range e.Warnings {
		s = p("%s. %s", s, warning)
	}

	return s
}

[NicolasMahe]

No I don't like this approach. I recommend to have small specific errors rather than a huge one

[ilgooz]

we had a further discussion about this on Discord. we decided to go with creating special error types for each.

as a future note if we face something like this again, my argument about this was:
using a generic error type can save us from doing multiple type assertions from error type. and these generic errors not that generic, they wraps the errors that are in same context.