Arvados

Shadowed variable bug.

16391-sbatch-error-logging @ 79d0b2480c207c12fd229403384881cd53a234e8 -- developer-run-tests: #1854

Isn't this update also shadowing the variable? Shouldn't be just "switch err.(type) {"?

diff --git a/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go b/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go
index 36bcef4f2..4115482d8 100644
--- a/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go
+++ b/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go
@@ -280,7 +280,8 @@ func (disp *Dispatcher) runContainer(_ *dispatch.Dispatcher, ctr arvados.Contain
                cmd = append(cmd, disp.cluster.Containers.CrunchRunArgumentsList...)
                if err := disp.submit(ctr, cmd); err != nil {
                        var text string
-                       if err, ok := err.(dispatchcloud.ConstraintsNotSatisfiableError); ok {
+                       switch err := err.(type) {
+                       case dispatchcloud.ConstraintsNotSatisfiableError:
                                var logBuf bytes.Buffer
                                fmt.Fprintf(&logBuf, "cannot run container %s: %s\n", ctr.UUID, err)
                                if len(err.AvailableTypes) == 0 {
@@ -296,7 +297,7 @@ func (disp *Dispatcher) runContainer(_ *dispatch.Dispatcher, ctr arvados.Contain
                                }
                                text = logBuf.String()
                                disp.UpdateState(ctr.UUID, dispatch.Cancelled)
-                       } else {
+                       default:
                                text = fmt.Sprintf("Error submitting container %s to slurm: %s", ctr.UUID, err)
                        }
                        log.Print(text)

Isn't this update also shadowing the variable? Shouldn't be just "switch err.(type) {"?

Yes and no: err is shadowed by a copy of the original err. switch err.(type) wouldn't shadow err with the specific-type value, so err.AvailableTypes wouldn't compile.

(In the previous code, the type assertion would return either err,true or nil,false, so the "else" case would always have err==nil instead of [a copy of] the original err.)

https://tour.golang.org/methods/16 https://play.golang.org/p/P88oNG4IP9u

Thanks for the example, this LGTM!