mc heal and mc mirror should respond with exit code (0) incase of errors

[Praveenrajmani]

mc heal will now respond with proper error on client.Heal
Will also add the fix for mc mirror (#2474)

Tested By

• ./mc admin heal --debug -r none; echo $?
• ./mc mirror --quiet foo play/testbucket; echo $?

Fixes #2534
Fixes #2474

[vadmeste]

LGTM

[kannappanr]

@Praveenrajmani Is this still WIP?

[Praveenrajmani]

Yes . i thought of fixing the mc mirror also in the same PR. Need some time for that

[codecov]

Codecov Report

Merging #2543 into master will not change coverage.
The diff coverage is 0%.

@@           Coverage Diff           @@
##           master    #2543   +/-   ##
=======================================
  Coverage   10.63%   10.63%           
=======================================
  Files         106      106           
  Lines       10703    10703           
=======================================
  Hits         1138     1138           
  Misses       9415     9415           
  Partials      150      150

Impacted Files	Coverage Δ
cmd/admin-heal.go	0% <0%> (ø)	⬆️
cmd/typed-errors.go	7.31% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c9f3369...3c0f142. Read the comment docs.

[Praveenrajmani]

Done and ready for review

[kannappanr]

@poornas @vadmeste please re-review the PR again

[vadmeste]

@Praveenrajmani if you see little below, the code already returns err.Trace() for errors that we don't ignore.

What's happening is that the code is always running case overwriteNotAllowedErr even for bucket is not found error.

The reason is that overwriteNotAllowedErr satisfies error interface that's why that switch case is always executed.

So to avoid this behavior, you can apply the following diff:

diff --git a/cmd/typed-errors.go b/cmd/typed-errors.go
index fd1590e8..50665be4 100644
--- a/cmd/typed-errors.go
+++ b/cmd/typed-errors.go
@@ -96,11 +96,13 @@ var errInvalidTarget = func(URL string) *probe.Error {
        return probe.NewError(invalidTargetErr(errors.New(msg))).Untrace()
 }

-type overwriteNotAllowedErr error
+type overwriteNotAllowedErr struct {
+       error
+}

 var errOverWriteNotAllowed = func(URL string) *probe.Error {
        msg := "Overwrite not allowed for `" + URL + "`. Use `--overwrite` to override this behavior."
-       return probe.NewError(overwriteNotAllowedErr(errors.New(msg)))
+       return probe.NewError(overwriteNotAllowedErr{errors.New(msg)})
 }

 type sourceIsDirErr error

So instead of adding this code, you can modify overwriteNotAllowedErr from interface to structure:

[Praveenrajmani]

Yes overwriteNotAllowedErr in switch case acts as default since it is an interface that returns an error , So the idea was to string check.
This seems better though , will change it

[vadmeste]

One small change needed

[harshavardhana]

This is not correct style expected style is to avoid string matching instead rely on types.

switch err.ToGoError().(type) {
case BucketDoesNotExist:

[Praveenrajmani]

Yes will change it to a struct with a error as Anis suggested

[Praveenrajmani]

Changes addressed .. Ready for re-review

[vadmeste]

LGTM