-
-
Notifications
You must be signed in to change notification settings - Fork 600
[Enhancement]: support errors from container option #2266
Description
Proposal
The current version of ContainerCustomizer provides the ability to customise a container in setup but as doesn't return an error so methods which do need to handle errors either have to panic or just the log the error and hope the user checks the log file, neither of which is a good user experience.
I propose we introduce a new type and interface that allows consumers to gracefully upgrade to a flow which will allow handling of errors from container customisation.
For example:
// SafeContainerCustomizer is an interface that can be used to customize a
// testcontainers container request and return an error if the customization
// fails.
type SafeContainerCustomizer interface {
// SafeCustomize applies the option to the container request and returns an
// error if the customization fails.
SafeCustomize(req *GenericContainerRequest) error
}
// SafeContainerOption is a function that modifies a container request.
type SafeContainerOption func(req *GenericContainerRequest) error
// SafeCustomize implements the SafeContainerCustomizer interface.
func (opt SafeContainerOption) SafeCustomize(req *GenericContainerRequest) error {
return opt(req)
}
// Customize implements the ContainerCustomizer interface.
// It panics if the customization fails.
// Deprecated: Use CustomizeSafe instead.
func (opt SafeContainerOption) Customize(req *GenericContainerRequest) {
if err := opt(req); err != nil {
panic(err)
}
}This will allow options which can encounter an error to switch from returning a ContainerOption to SafeContainerOption in a way which is compatible.
Handlers would need to check if the type implements SafeContainerOption and use it in preference for example:
for _, opt := range options {
if safeOpt, ok := opt.(testcontainers.SafeContainerCustomizer); ok {
if err := safeOpt.SafeCustomize(&req); err != nil {
return nil, err
}
} else {
opt.Customize(&req)
}
}We could change the ContainerCustomizer, given testcontainers is still pre v1,but not a nice user experience, but it should be removed in v2 in my opinion.
Thoughts?
Happy to do implement if folks agree.