Skip to content

Commit

Permalink
sync cmd: Add better error handling for unexpected cases
Browse files Browse the repository at this point in the history
  • Loading branch information
@andygrunwald
andygrunwald committed Sep 13, 2024
1 parent 0d291c0 commit 91b7d36
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 45 deletions.
44 changes: 33 additions & 11 deletions cmd/sync.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,19 +282,28 @@ func runRecipes(p *tea.Program, logger *slog.Logger, supplier, localOICDBChecksu
case "browser":
browserDriver, err := browser.NewBrowserDriver(logger, recipeCredentials, buchhalterDocumentsDirectory, documentArchive, buchhalterMaxDownloadFilesPerReceipt)
if err != nil {
logger.Error("Error initializing a new browser driver", "error", err, "supplier", recipesToExecute[i].recipe.Supplier)
p.Send(utils.ViewStatusUpdateMsg{
Err: fmt.Errorf("error downloading invoices from %s: %w", recipesToExecute[i].recipe.Supplier, err),
Err: fmt.Errorf("error initializing a new browser driver for supplier `%s`: %w", recipesToExecute[i].recipe.Supplier, err),
Completed: true,
})
// TODO Should we continue here or abort? !!!
// We skip this supplier and continue with the next one
continue
}

// Send the browser context to the view layer
// This is needed in case of an external abort signal (e.g. CTRL+C).
p.Send(updateBrowserContext{ctx: browserDriver.GetContext()})

// TODO Catch error here
recipeResult = browserDriver.RunRecipe(p, totalStepCount, stepCountInCurrentRecipe, baseCountStep, recipesToExecute[i].recipe)
recipeResult, err = browserDriver.RunRecipe(p, totalStepCount, stepCountInCurrentRecipe, baseCountStep, recipesToExecute[i].recipe)
if err != nil {
logger.Error("Error running browser recipe", "error", err, "supplier", recipesToExecute[i].recipe.Supplier)
p.Send(utils.ViewStatusUpdateMsg{
Err: fmt.Errorf("error running browser recipe for supplier `%s`: %w", recipesToExecute[i].recipe.Supplier, err),
Completed: true,
})
// We fall through, because recipeResult might be set and contains additional information
}
if ChromeVersion == "" {
ChromeVersion = browserDriver.ChromeVersion
}
Expand All @@ -306,19 +315,28 @@ func runRecipes(p *tea.Program, logger *slog.Logger, supplier, localOICDBChecksu
case "client":
clientDriver, err := browser.NewClientAuthBrowserDriver(logger, recipeCredentials, buchhalterConfigDirectory, buchhalterDocumentsDirectory, documentArchive)
if err != nil {
logger.Error("Error initializing a new client auth browser driver", "error", err, "supplier", recipesToExecute[i].recipe.Supplier)
p.Send(utils.ViewStatusUpdateMsg{
Err: fmt.Errorf("error downloading invoices from %s: %w", recipesToExecute[i].recipe.Supplier, err),
Err: fmt.Errorf("error initializing a new client auth browser for supplier `%s`: %w", recipesToExecute[i].recipe.Supplier, err),
Completed: true,
})
// TODO Should we continue here or abort? !!!
// We skip this supplier and continue with the next one
continue
}

// Send the browser context to the view layer
// This is needed in case of an external abort signal (e.g. CTRL+C).
p.Send(updateBrowserContext{ctx: clientDriver.GetContext()})

// TODO Catch error here
recipeResult = clientDriver.RunRecipe(p, totalStepCount, stepCountInCurrentRecipe, baseCountStep, recipesToExecute[i].recipe)
recipeResult, err = clientDriver.RunRecipe(p, totalStepCount, stepCountInCurrentRecipe, baseCountStep, recipesToExecute[i].recipe)
if err != nil {
logger.Error("Error running browser recipe", "error", err, "supplier", recipesToExecute[i].recipe.Supplier)
p.Send(utils.ViewStatusUpdateMsg{
Err: fmt.Errorf("error running browser recipe for supplier `%s`: %w", recipesToExecute[i].recipe.Supplier, err),
Completed: true,
})
// We fall through, because recipeResult might be set and contains additional information
}
if ChromeVersion == "" {
ChromeVersion = clientDriver.ChromeVersion
}
Expand All @@ -328,6 +346,8 @@ func runRecipes(p *tea.Program, logger *slog.Logger, supplier, localOICDBChecksu
// In case of an external abort signal (e.g. CTRL+C), bubbletea will call `chromedp.Cancel()`.
}

// TODO recipeResult can be empty! (not nil, but without values)

rdx := repository.RunDataSupplier{
Supplier: recipesToExecute[i].recipe.Supplier,
Version: recipesToExecute[i].recipe.Version,
Expand All @@ -338,7 +358,6 @@ func runRecipes(p *tea.Program, logger *slog.Logger, supplier, localOICDBChecksu
}
RunData = append(RunData, rdx)

// TODO Check for recipeResult.LastErrorMessage
p.Send(viewMsgRecipeDownloadResultMsg{
duration: time.Since(startTime),
newFilesCount: recipeResult.NewFilesCount,
Expand Down Expand Up @@ -391,7 +410,7 @@ func runRecipes(p *tea.Program, logger *slog.Logger, supplier, localOICDBChecksu
logger.Info("Uploading document to Buchhalter API ...", "file", fileInfo.Path, "checksum", fileChecksum)
result, err := buchhalterAPIClient.DoesDocumentExist(fileChecksum)
if err != nil {
// TODO Implement better error handling
// Skip the file if we can't check the existence of the document in the API
logger.Error("Error checking if document exists already in Buchhalter API", "file", fileInfo.Path, "checksum", fileChecksum, "error", err)
continue
}
Expand All @@ -405,7 +424,10 @@ func runRecipes(p *tea.Program, logger *slog.Logger, supplier, localOICDBChecksu

err = buchhalterAPIClient.UploadDocument(fileInfo.Path, fileInfo.Supplier)
if err != nil {
// TODO Implement better error handling
p.Send(utils.ViewStatusUpdateMsg{
Err: fmt.Errorf("error uploading document `%s` from `%s` to Buchhalter API: %w", fileInfo.Path, fileInfo.Supplier, err),
Completed: true,
})
logger.Error("Error uploading document to Buchhalter API", "file", fileInfo.Path, "supplier", fileInfo.Supplier, "error", err)
continue
}
Expand Down
48 changes: 25 additions & 23 deletions lib/browser/browser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,11 @@ type BrowserDriver struct {
documentArchive *archive.DocumentArchive

buchhalterDocumentsDirectory string
downloadsDirectory string
documentsDirectory string

ChromeVersion string

// TODO Check if those are needed
downloadsDirectory string
documentsDirectory string

browserCtx context.Context
browserCancel context.CancelFunc
recipeTimeout time.Duration
Expand Down Expand Up @@ -97,7 +95,7 @@ func (b *BrowserDriver) GetContext() context.Context {
return b.browserCtx
}

func (b *BrowserDriver) RunRecipe(p *tea.Program, totalStepCount int, stepCountInCurrentRecipe int, baseCountStep int, recipe *parser.Recipe) utils.RecipeResult {
func (b *BrowserDriver) RunRecipe(p *tea.Program, totalStepCount int, stepCountInCurrentRecipe int, baseCountStep int, recipe *parser.Recipe) (utils.RecipeResult, error) {
b.logger.Info("Starting chrome browser driver ...", "recipe", recipe.Supplier, "recipe_version", recipe.Version)

ctx := b.browserCtx
Expand All @@ -111,19 +109,25 @@ func (b *BrowserDriver) RunRecipe(p *tea.Program, totalStepCount int, stepCountI
chromedp.Text(`#version`, &b.ChromeVersion, chromedp.NodeVisible),
})
if err != nil {
// TODO Implement error handling
panic(err)
b.logger.Error("Error while determining the Chrome version", "error", err.Error())
p.Send(utils.ViewStatusUpdateMsg{
Err: fmt.Errorf("error while determining the Chrome version: %w", err),
Completed: true,
})
// We fall through here, because we can still continue without the Chrome version
}
b.ChromeVersion = strings.TrimSpace(b.ChromeVersion)
}
b.logger.Info("Starting chrome browser driver ... completed ", "recipe", recipe.Supplier, "recipe_version", recipe.Version, "chrome_version", b.ChromeVersion)

var result utils.RecipeResult

// Create download directories
var err error
b.downloadsDirectory, b.documentsDirectory, err = utils.InitSupplierDirectories(b.buchhalterDocumentsDirectory, recipe.Supplier)
if err != nil {
// TODO Implement error handling
fmt.Println(err)
b.logger.Error("Error while creating download directory", "error", err.Error(), "documents_directory", b.buchhalterDocumentsDirectory, "supplier", recipe.Supplier)
return result, fmt.Errorf("error while creating download directory: %w", err)
}
b.logger.Info("Download directories created", "downloads_directory", b.downloadsDirectory, "documents_directory", b.documentsDirectory)

Expand All @@ -139,8 +143,8 @@ func (b *BrowserDriver) RunRecipe(p *tea.Program, totalStepCount int, stepCountI
}),
})
if err != nil {
// TODO Implement error handling
panic(err)
b.logger.Error("Error while configuring the download behavior of chrome", "error", err.Error(), "downloads_directory", b.downloadsDirectory)
return result, fmt.Errorf("error while configuring the download behavior of chrome: %w", err)
}

// Disable downloading images for performance reasons
Expand All @@ -150,7 +154,6 @@ func (b *BrowserDriver) RunRecipe(p *tea.Program, totalStepCount int, stepCountI

var cs float64
n := 1
var result utils.RecipeResult
for _, step := range recipe.Steps {
p.Send(utils.ViewStatusUpdateMsg{
Message: fmt.Sprintf("Downloading invoices from `%s` (%d/%d):", recipe.Supplier, n, stepCountInCurrentRecipe),
Expand All @@ -163,7 +166,6 @@ func (b *BrowserDriver) RunRecipe(p *tea.Program, totalStepCount int, stepCountI
if step.When.URL != "" {
var currentURL string
if err := chromedp.Run(ctx, chromedp.Location(&currentURL)); err != nil {
// TODO implement better error handling
b.logger.Error("Failed to get current URL", "error", err.Error())

// Skipping step
Expand Down Expand Up @@ -236,10 +238,10 @@ func (b *BrowserDriver) RunRecipe(p *tea.Program, totalStepCount int, stepCountI
}
err = utils.TruncateDirectory(b.downloadsDirectory)
if err != nil {
// TODO Implement error handling
fmt.Println(err)
b.logger.Error("Error while truncating the download directory", "error", err.Error(), "downloads_directory", b.downloadsDirectory)
return result, fmt.Errorf("error while truncating the download directory: %w", err)
}
return result
return result, nil
}

case <-time.After(b.recipeTimeout):
Expand All @@ -253,16 +255,16 @@ func (b *BrowserDriver) RunRecipe(p *tea.Program, totalStepCount int, stepCountI
}
err = utils.TruncateDirectory(b.downloadsDirectory)
if err != nil {
// TODO Implement error handling
fmt.Println(err)
b.logger.Error("Error while truncating the download directory", "error", err.Error(), "downloads_directory", b.downloadsDirectory)
return result, fmt.Errorf("error while truncating the download directory: %w", err)
}

// Imagine we run the `downloadALl` step, we download 2 files and then the recipe times out.
// Imagine we run the `downloadAll` step, we download 2 files and then the recipe times out.
// It is bad that the recipe timed out, however, we still want to process with the 2 new downloaded documents.
// Process in this context means to move the files to the documents directory and add them to the document archive.
// Thats why we don't abort if the recipe timed out in this stage.
if !(step.Action == "downloadAll" && b.downloadedFilesCount > 0) {
return result
return result, nil
}
}
cs = (float64(baseCountStep) + float64(n)) / float64(totalStepCount)
Expand All @@ -272,10 +274,10 @@ func (b *BrowserDriver) RunRecipe(p *tea.Program, totalStepCount int, stepCountI

err = utils.TruncateDirectory(b.downloadsDirectory)
if err != nil {
// TODO Implement error handling
fmt.Println(err)
b.logger.Error("Error while truncating the download directory", "error", err.Error(), "downloads_directory", b.downloadsDirectory)
return result, fmt.Errorf("error while truncating the download directory: %w", err)
}
return result
return result, nil
}

func (b *BrowserDriver) stepOpen(ctx context.Context, step parser.Step) utils.StepResult {
Expand Down
23 changes: 14 additions & 9 deletions lib/browser/oauth2.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func (b *ClientAuthBrowserDriver) GetContext() context.Context {
return b.browserCtx
}

func (b *ClientAuthBrowserDriver) RunRecipe(p *tea.Program, totalStepCount int, stepCountInCurrentRecipe int, baseCountStep int, recipe *parser.Recipe) utils.RecipeResult {
func (b *ClientAuthBrowserDriver) RunRecipe(p *tea.Program, totalStepCount int, stepCountInCurrentRecipe int, baseCountStep int, recipe *parser.Recipe) (utils.RecipeResult, error) {
b.logger.Info("Starting client auth chrome browser driver ...", "recipe", recipe.Supplier, "recipe_version", recipe.Version)

ctx := b.browserCtx
Expand All @@ -117,25 +117,30 @@ func (b *ClientAuthBrowserDriver) RunRecipe(p *tea.Program, totalStepCount int,
chromedp.Text(`#version`, &b.ChromeVersion, chromedp.NodeVisible),
})
if err != nil {
// TODO Implement error handling
panic(err)
b.logger.Error("Error while determining the Chrome version", "error", err.Error())
p.Send(utils.ViewStatusUpdateMsg{
Err: fmt.Errorf("error while determining the Chrome version: %w", err),
Completed: true,
})
// We fall through here, because we can still continue without the Chrome version
}
b.ChromeVersion = strings.TrimSpace(b.ChromeVersion)
}
b.logger.Info("Starting client auth chrome browser driver ... completed ", "recipe", recipe.Supplier, "recipe_version", recipe.Version, "chrome_version", b.ChromeVersion)

var result utils.RecipeResult

// Create download directories
var err error
b.downloadsDirectory, b.documentsDirectory, err = utils.InitSupplierDirectories(b.buchhalterDocumentsDirectory, recipe.Supplier)
if err != nil {
// TODO Implement error handling
fmt.Println(err)
b.logger.Error("Error while creating download directory", "error", err.Error(), "documents_directory", b.buchhalterDocumentsDirectory, "supplier", recipe.Supplier)
return result, err
}
b.logger.Info("Download directories created", "downloads_directory", b.downloadsDirectory, "documents_directory", b.documentsDirectory)

var cs float64
n := 1
var result utils.RecipeResult
for _, step := range recipe.Steps {
p.Send(utils.ViewStatusUpdateMsg{
Message: fmt.Sprintf("Downloading invoices from %s (%d/%d):", recipe.Supplier, n, stepCountInCurrentRecipe),
Expand Down Expand Up @@ -186,7 +191,7 @@ func (b *ClientAuthBrowserDriver) RunRecipe(p *tea.Program, totalStepCount int,
NewFilesCount: b.newFilesCount,
}
if lastStepResult.Break {
return result
return result, nil
}
}

Expand All @@ -199,15 +204,15 @@ func (b *ClientAuthBrowserDriver) RunRecipe(p *tea.Program, totalStepCount int,
LastStepDescription: step.Description,
NewFilesCount: b.newFilesCount,
}
return result
return result, nil
}

cs = (float64(baseCountStep) + float64(n)) / float64(totalStepCount)
p.Send(utils.ViewProgressUpdateMsg{Percent: cs})
n++
}

return result
return result, nil
}

func (b *ClientAuthBrowserDriver) stepOauth2Setup(step parser.Step) utils.StepResult {
Expand Down
2 changes: 0 additions & 2 deletions lib/parser/parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ type Database struct {
}

type Recipe struct {
// TODO Rename Prodiver to Supplier
Supplier string `json:"supplier"`
Domains []string `json:"domains"`
Version string `json:"version"`
Expand Down Expand Up @@ -169,7 +168,6 @@ func validateRecipes(buchhalterConfigDirectory string) (bool, error) {
return true, nil
}

// TODO Create our own error type here (validation error)
errorMessageParts := []string{}
for _, errorDescription := range result.Errors() {
errorMessageParts = append(errorMessageParts, errorDescription.String())
Expand Down

0 comments on commit 91b7d36

Please sign in to comment.