Update training.go

[m110]

Thank you @blaggacao!

[blaggacao]

I wanted to submit two more, but I'm still in doubt:

https://github.com/ThreeDotsLabs/wild-workouts-go-ddd-example/blob/186a2c4a912e485ac7bb4d18c2892df7617e9ec9/internal/trainings/domain/training/training.go#L90

• note-too-long slug seems to be domain knowledge of the client implementation not of the training domain, but I haven't figured out how to decouple them without destroying DRY.

https://github.com/ThreeDotsLabs/wild-workouts-go-ddd-example/blob/186a2c4a912e485ac7bb4d18c2892df7617e9ec9/internal/trainings/domain/training/repository.go#L8-L14

• NotFoundError seems to be a pretty generic pattern that a domain needs to know from the repository, so I was trying to model it after the behavioural error pattern of https://dave.cheney.net/2016/04/27/dont-just-check-errors-handle-them-gracefully with an NotFoundUUID() anonymous interface, but it didn't feel right without the correct integrated reasoning, so I gave up.

// domain/x/repository.go
func notFoundUUID(err error) string {
        if i, ok := err.(interface{NotFoundUUID() string}); ok {
                return i.NotFoundUUID()
        }
        return ""
}

That's not really behavioural, though, maybe a better behavioural pattern would be DomainShouldHandleThisError(), but immediately follows the question: well, how?

[m110]

Hey @blaggacao.

note-too-long slug seems to be domain knowledge of the client implementation not of the training domain, but I haven't figured out how to decouple them without destroying DRY.

I'm not sure I see your point, do you mean this is the logic of the frontend application? I think this rule is fine to be enforced on the domain level. Maybe in this case it looks silly, but it could easily be the length of the username, which has more serious consequences.

As for the NotFoundError, note that this error is defined in the domain! It's also not just any generic NotFound error, but a "training not found error". Note the package name in trainings_firestore_repository.go:

    if status.Code(err) == codes.NotFound {
        return nil, training.NotFoundError{trainingUUID}
    }

So it's not the domain that knows about a database error. It's the repository that maps the error to the proper domain-specific error.

Does that make sense? 🙂

[blaggacao]

I'm not sure I see your point, do you mean this is the logic of the frontend application?

Correct. My impression is that note-too-long is the string identifier of a translation value (or even an additional client side string sentinel), while Note too long is the genuine message of this error instance.

While the former is paying tribute to the frontend scope, the letter is genuin domain logic. I tried to tear those aspects apart, but it didn't get any more readable. Maybe I didn't try hard enough.

Thingking over it again, maybe the sentinel error instance approach (instead of current string sentinel approach) could work.

It would also ensure that translation string sentinel note-too-long is moved closer to the translation implementation.

var ErrNoteTooLong = fmt.Errorf("Note too long")

and at some translation middleware before going on wire to the frontend client:

if errors.Is(err, ErrNoteTooLong) {
    // go look for translation with slug note-too-long
    // or instantiate a slug error for further client side use
}

---

Re: NotFoundError

It is a contract requirement from the domain scope, indeed. My line of thinking was that it actually can be contract offering of any type of repository implementation for any domain.

To circumvent the problem that a domain cannot know specifics of the repository, we'd need a contract broker, like ./common or alternative.

Or, the domain could express it's expectation towards the contract by requiring an interface to be met which indicates that the value was not found and which is the uuid of that value.

Yet, as you can see in my comment above, the resulting interface casting code is not very readable for the readability requirements of the domain's code.

To be really up to the linked philosophy on behavioural error handling, this needs some further thought, thoug (and validation if this academic discussion also worthwhile, not only enjoyable).

Maybe the domain(s) could expect from their repository(ies) that every error is decorated with a IsNotFound() method, similarily IsAlreadyExists() and other generic return aspects during the intercom between domain and repo.

a, err := GetAccountByUuid(...)
if err.IsNotFound() {
    // Try again with alternative
} else if err != nil {
    panic(err)
}

Thank you for this discussion! We can let it settle for a while, maybe something interesting evolves out of it with time.

[m110]

Hey @blaggacao!

Alright, I see your point now. 👍 I think the error.Is check could work fine, if you'd like to extract the slugs out of the domain.

However, note that this is just a Slug, not a TranslationSlug. It's like a unique identifier of the error that can be used outside of the domain. We could also use it in another service to check the error cause, for instance.

We could also map it on the ports layer, as you mentioned. I'm trying to come up with some issues this could make living in the domain, but nothing comes to mind at the moment. Please share if you thought of something. 🙂

---

As for the NotFoundError, I think what you mean is, you'd like to ensure that every repository implementation returns this specific error when the entity is not found, correct? If you have multiple repository implementations, you could keep a generic test suite that all of them would need to pass. We've used a similar approach in watermill Pub/Sub tests: https://github.com/ThreeDotsLabs/watermill/blob/master/pubsub/tests/test_pubsub.go#L38

[blaggacao]

However, note that this is just a Slug, not a TranslationSlug. It's like a unique identifier of the error that can be used outside of the domain.

Excellent point. (... and outside of go — that clicked!)

Maybe a more lightweight approach than Slug could be a stringer (generated) in order to be able to wire the error type appropriately.

There is no real issue at all with the current implementation. I'm doing this for the joy of understanding. ;-) Still a stringer might probably be a more idiomatic approach (no extra import, available code generators).

---

As for the NotFoundError, I think what you mean is, you'd like to ensure that every repository implementation returns this specific error when the entity is not found, correct?

Exactly, except instead of return a specific error satisfying an interface so we can save another import statement. This interface would reveal error categories that domains usually care. I believe, those error categories are a finit well known set of categories (like not found, already exists, locked — but not general database server errors)

[m110]

Yeah, a Stringer could work as well. :) We like to separate the slug from the error message, as the slug becomes part of the contract with outside world and should stay immutable. You can change the message however, if you'd like more descriptive errors for developers.

[m110]

Actually, the comment above states where's the issue with this approach:

the slug becomes part of the contract with outside world

So yes, it might be better to map the errors to slugs in ports, as currently changes in domain may impact the contract, which doesn't sound good. 🙂

[blaggacao]

We like to separate the slug from the error message, as the slug becomes part of the contract with outside world and should stay immutable.

For a stringer, I would implement reflection on the Identifier (ErrNoteTooLong). A custom stringer could even do: "err-note-too-long"

the slug becomes part of the contract with outside world

Yep, in addition to the ErrNoteTooLong instance (with the go outside world), so a stringer that acts on the type identifier would make this identity relationship transparent. With current explicit string-slug, it's a redundant item on the contract inventory. Redundancy can lead to inconsistencies, for example when two go types start to identify by the same slug. Then we have concurring superposition of public apis.

[blaggacao]

Based on https://dave.cheney.net/2019/06/10/constant-time I think found a good replacement for the slug errors.

// errors.go
package scheduler

type schedulerError string
func (e schedulerError) Error() string { return string(e) }

const (
    ErrAgentAlreadyAssigned = schedulerError("agent-already-assigned")
)

• Constants
• Can be wrapped with something like fmt.Errorf("agent: %s; scheduler: %s scheduler: %w", a,s, ErrAgentAlreadyAssigned) for extra context
• Probing the sentinel errors.Is(... , ErrAgentAlreadyAssigned) works as expected
• "Getting" the slug: slug := ErrAgentAlreadyAssigned