Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove loggedError #94

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/container-suseconnect/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func actionWrapper(action func(*cli.Context) error) func(*cli.Context) error {
if err := action(ctx); err != nil {
switch err.(type) {
case *cs.SuseConnectError:
if err.(*cs.SuseConnectError).ErrorCode == cs.GetCredentialsError {
if err.(*cs.SuseConnectError).Code == cs.GetCredentialsError {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the plan is to use fmt.Errorf you can use errors.Is (or errors.As) if you use %w.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is going away with #93, this PR needs a rebase once that is merged.

if ctx.Bool("log-credentials-errors") {
return err
}
Expand Down
8 changes: 4 additions & 4 deletions internal/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,12 @@ func ReadConfiguration(config Configuration) error {
if config.onLocationsNotFound() {
return nil
}
return loggedError(GetCredentialsError, "Warning: SUSE credentials not found: %v - automatic handling of repositories not done.", config.locations())
return NewSuseConnectError(GetCredentialsError, "Warning: SUSE credentials not found: %v - automatic handling of repositories not done.", config.locations())
}

file, err := os.Open(path)
if err != nil {
return loggedError(GetCredentialsError, "Can't open %s file: %v", path, err.Error())
return NewSuseConnectError(GetCredentialsError, "Can't open %s file: %v", path, err.Error())
}
defer file.Close()

Expand All @@ -96,7 +96,7 @@ func parse(config Configuration, reader io.Reader) error {
line := scanner.Text()
parts := strings.SplitN(line, string(config.separator()), 2)
if len(parts) != 2 {
return loggedError(GetCredentialsError, "Can't parse line: %v", line)
return NewSuseConnectError(GetCredentialsError, "Can't parse line: %v", line)
}

// And finally trim the key and the value and pass it to the config.
Expand All @@ -106,7 +106,7 @@ func parse(config Configuration, reader io.Reader) error {

// Final checks.
if err := scanner.Err(); err != nil {
return loggedError(GetCredentialsError, "Error when scanning configuration: %v", err)
return NewSuseConnectError(GetCredentialsError, "Error when scanning configuration: %v", err)
}
if err := config.afterParseCheck(); err != nil {
return err
Expand Down
4 changes: 0 additions & 4 deletions internal/configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ func TestNotFound(t *testing.T) {
if err == nil || err.Error() != "Warning: SUSE credentials not found: [] - automatic handling of repositories not done." {
t.Fatalf("Wrong error: %v", err)
}
shouldHaveLogged(t, "Warning: SUSE credentials not found: [] - automatic handling of repositories not done.")
}

type NotAllowedConfiguration struct{}
Expand Down Expand Up @@ -99,7 +98,6 @@ func TestNotAllowed(t *testing.T) {
if err == nil || err.Error() != msg {
t.Fatal("Wrong error")
}
shouldHaveLogged(t, msg)
}

func TestParseInvalid(t *testing.T) {
Expand All @@ -116,7 +114,6 @@ func TestParseInvalid(t *testing.T) {
if err == nil || err.Error() != msg {
t.Fatal("Wrong error")
}
shouldHaveLogged(t, msg)
}

type ErrorAfterParseConfiguration struct{}
Expand Down Expand Up @@ -160,5 +157,4 @@ func TestParseFailNoSeparator(t *testing.T) {
if err == nil || err.Error() != msg {
t.Fatal("Wrong error")
}
shouldHaveLogged(t, msg)
}
4 changes: 2 additions & 2 deletions internal/credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,10 @@ func (cr *Credentials) setValues(key, value string) {

func (cr *Credentials) afterParseCheck() error {
if cr.Username == "" {
return loggedError(GetCredentialsError, "Can't find username")
return NewSuseConnectError(GetCredentialsError, "Can't find username")
}
if cr.Password == "" {
return loggedError(GetCredentialsError, "Can't find password")
return NewSuseConnectError(GetCredentialsError, "Can't find password")
}
return nil
}
2 changes: 0 additions & 2 deletions internal/credentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ func TestCredentials(t *testing.T) {
if err == nil || err.Error() != msg {
t.Fatal("Wrong error")
}
shouldHaveLogged(t, msg)

cr.setValues("username", "suse")
prepareLogger()
Expand All @@ -42,7 +41,6 @@ func TestCredentials(t *testing.T) {
if err == nil || err.Error() != msg {
t.Fatal("Wrong error")
}
shouldHaveLogged(t, msg)

cr.setValues("password", "1234")
err = cr.afterParseCheck()
Expand Down
22 changes: 19 additions & 3 deletions internal/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@

package containersuseconnect

import (
"fmt"
)

const (
// GetCredentialsError indicates a failure to retrieve or parse
// credentials
Expand All @@ -36,10 +40,22 @@ const (
// SuseConnectError is a custom error type allowing us to distinguish between
// different error kinds via the `ErrorCode` field
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// different error kinds via the `ErrorCode` field
// different error kinds via the `Code` field

type SuseConnectError struct {
ErrorCode int
message string
Code int
Err error
}

func (s *SuseConnectError) Error() string {
return s.message
return s.Err.Error()
}

func (s *SuseConnectError) Unwrap() error {
return s.Err
}

func NewSuseConnectError(errorCode int, format string, params ...interface{}) *SuseConnectError {
err := fmt.Errorf(format, params...)
return &SuseConnectError{
Code: errorCode,
Err: err,
}
}
6 changes: 3 additions & 3 deletions internal/installed_product.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,21 +61,21 @@ func parseInstalledProduct(reader io.Reader) (InstalledProduct, error) {
err := xml.Unmarshal(xmlData, &p)
if err != nil {
return InstalledProduct{},
loggedError(InstalledProductError, "Can't parse base product file: %v", err.Error())
NewSuseConnectError(InstalledProductError, "Can't parse base product file: %v", err.Error())
}
return p, nil
}

// Read the product file from the standard location
func readInstalledProduct(provider ProductProvider) (InstalledProduct, error) {
if _, err := os.Stat(provider.Location()); os.IsNotExist(err) {
return InstalledProduct{}, loggedError(InstalledProductError, "No base product detected")
return InstalledProduct{}, NewSuseConnectError(InstalledProductError, "No base product detected")
}

xmlFile, err := os.Open(provider.Location())
if err != nil {
return InstalledProduct{},
loggedError(InstalledProductError, "Can't open base product file: %v", err.Error())
NewSuseConnectError(InstalledProductError, "Can't open base product file: %v", err.Error())
}
defer xmlFile.Close()

Expand Down
13 changes: 0 additions & 13 deletions internal/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
package containersuseconnect

import (
"fmt"
"log"
"os"
)

Expand Down Expand Up @@ -47,14 +45,3 @@ func GetLoggerFile() *os.File {
}
return os.Stderr
}

// Log the given formatted string with its parameters, and return it
// as a new error.
func loggedError(errorCode int, format string, params ...interface{}) *SuseConnectError {
msg := fmt.Sprintf(format, params...)
log.Print(msg)
return &SuseConnectError{
ErrorCode: errorCode,
message: msg,
}
}
11 changes: 5 additions & 6 deletions internal/products.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ func fixRepoUrlsForRMT(p *Product) error {
for i := range p.Repositories {
repourl, err := url.Parse(p.Repositories[i].URL)
if err != nil {
loggedError(RepositoryError, "Unable to parse repository URL: %s - %v", p.Repositories[i].URL, err)
return err
return NewSuseConnectError(RepositoryError, "Unable to parse repository URL: %s - %v", p.Repositories[i].URL, err)
}
params := repourl.Query()
if params.Get("credentials") == "" {
Expand All @@ -83,7 +82,7 @@ func parseProducts(reader io.Reader) ([]Product, error) {
data, err := io.ReadAll(reader)
if err != nil {
return products,
loggedError(RepositoryError, "Can't read product information: %v", err.Error())
NewSuseConnectError(RepositoryError, "Can't read product information: %v", err.Error())
}

// Depending on which API was used the JSON we get passed contains
Expand All @@ -106,7 +105,7 @@ func parseProducts(reader io.Reader) ([]Product, error) {
}
if err != nil {
return products,
loggedError(RepositoryError, "Can't read product information: %v - %s", err.Error(), data)
NewSuseConnectError(RepositoryError, "Can't read product information: %v - %s", err.Error(), data)
}
return products, nil
}
Expand All @@ -129,7 +128,7 @@ func requestProductsFromRegCodeOrSystem(data SUSEConnectData, regCode string,
req, err := http.NewRequest("GET", data.SccURL, nil)
if err != nil {
return products,
loggedError(NetworkError, "Could not connect with registration server: %v\n", err)
NewSuseConnectError(NetworkError, "Could not connect with registration server: %v\n", err)
}

values := req.URL.Query()
Expand Down Expand Up @@ -164,7 +163,7 @@ func requestProductsFromRegCodeOrSystem(data SUSEConnectData, regCode string,
}
}
return products,
loggedError(SubscriptionServerError, "Unexpected error while retrieving products with regCode %s: %s", regCode, resp.Status)
NewSuseConnectError(SubscriptionServerError, "Unexpected error while retrieving products with regCode %s: %s", regCode, resp.Status)
}

return parseProducts(resp.Body)
Expand Down
19 changes: 0 additions & 19 deletions internal/setup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ package containersuseconnect
import (
"bytes"
"log"
"regexp"
"strings"
"testing"
)

// Handy functions to be used by the test suite.
Expand All @@ -33,19 +30,3 @@ func prepareLogger() {
logged = bytes.NewBuffer([]byte{})
log.SetOutput(logged)
}

// Make sure that the logged string matches the given expected string.
func shouldHaveLogged(t *testing.T, str string) {
original := logged.String()
if strings.TrimSpace(original) == "" {
t.Fatal("Nothing has been logged.\n")
}

// The logged string includes the timestamp, get rid of it.
re := regexp.MustCompile("^\\d{4}/\\d{2}/\\d{2} \\d{2}:\\d{2}:\\d{2}\\s")
logStr := strings.TrimSpace(re.ReplaceAllString(original, ""))

if strings.TrimSpace(str) != logStr {
t.Fatalf("Should have logged: %v, not %v\n", str, logStr)
}
}
13 changes: 7 additions & 6 deletions internal/subscriptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func requestRegcodes(data SUSEConnectData, credentials Credentials) ([]string, e
req, err := http.NewRequest("GET", data.SccURL, nil)
if err != nil {
return codes,
loggedError(NetworkError, "Could not connect with registration server: %v\n", err)
NewSuseConnectError(NetworkError, "Could not connect with registration server: %v\n", err)
}

req.URL.Path = "/connect/systems/subscriptions"
Expand All @@ -71,7 +71,7 @@ func requestRegcodes(data SUSEConnectData, credentials Credentials) ([]string, e

if resp.StatusCode != 200 {
return codes,
loggedError(SubscriptionServerError, "Unexpected error while retrieving regcode: %s", resp.Status)
NewSuseConnectError(SubscriptionServerError, "Unexpected error while retrieving regcode: %s", resp.Status)
}

subscriptions, err := parseSubscriptions(resp.Body)
Expand All @@ -83,9 +83,10 @@ func requestRegcodes(data SUSEConnectData, credentials Credentials) ([]string, e
if strings.ToUpper(subscription.Status) != "EXPIRED" {
codes = append(codes, subscription.RegCode)
} else {
loggedError(SubscriptionServerError, "Skipping regCode: %s -- expired.", subscription.RegCode)
log.Println(NewSuseConnectError(SubscriptionServerError, "Skipping regCode: %s -- expired.", subscription.RegCode))
}
}

return codes, err
}

Expand All @@ -96,15 +97,15 @@ func parseSubscriptions(reader io.Reader) ([]Subscription, error) {

data, err := io.ReadAll(reader)
if err != nil {
return subscriptions, loggedError(SubscriptionError, "Can't read subscriptions information: %v", err.Error())
return subscriptions, NewSuseConnectError(SubscriptionError, "Can't read subscriptions information: %v", err.Error())
}

err = json.Unmarshal(data, &subscriptions)
if err != nil {
return subscriptions, loggedError(SubscriptionError, "Can't read subscription: %v", err.Error())
return subscriptions, NewSuseConnectError(SubscriptionError, "Can't read subscription: %v", err.Error())
}
if len(subscriptions) == 0 {
return subscriptions, loggedError(SubscriptionError, "Got 0 subscriptions")
return subscriptions, NewSuseConnectError(SubscriptionError, "Got 0 subscriptions")
}
return subscriptions, nil
}