upload: process counterflow messages

[bassosimone]

While there, also go get -u -v ./... && go mod tidy

---

This change is 

[stephen-soltesz]

Reviewed 1 of 3 files at r3.
Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @bassosimone)

---

internal/upload/upload.go, line 52 at r3 (raw file):

		err := conn.SetReadDeadline(time.Now().Add(params.UploadTimeout))
		if err != nil {
			errCh <- err

Previously this loop would return. Now it continues -- should this be continue? Or return the err and then return? or break? Are these errors recoverable?

Whatever the case, similar handling below.

---

internal/upload/upload.go, line 135 at r3 (raw file):

	ctx, cancel := context.WithTimeout(ctx, params.UploadTimeout)
	defer cancel()
	errCh := make(chan error)

Who closes this channel?

---

internal/upload/upload.go, line 148 at r3 (raw file):

	err := <-errCh
	if err != nil {

If Run returns on the first error from the readcounterflow goroutine, I would expect the goroutine to quit immediately.

[robertodauria]

Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @stephen-soltesz)

---

internal/upload/upload.go, line 52 at r3 (raw file):

Previously, stephen-soltesz (Stephen Soltesz) wrote…

Previously this loop would return. Now it continues -- should this be continue? Or return the err and then return? or break? Are these errors recoverable?

Whatever the case, similar handling below.

Good catch! These should never happen under normal operation.

• Setting a ReadDeadline should always be possible
• ReadMessage() shouldn't fail unless the connection was dropped
• The server should never send anything other than TextMessages during the upload
• The JSON from the server should be a valid spec.Measurement in every case

I would prefer to let the upload fail, so it's obvious something is wrong (either on the server or something strange between client and server or the connection was dropped).

---

internal/upload/upload.go, line 135 at r3 (raw file):

Previously, stephen-soltesz (Stephen Soltesz) wrote…

Who closes this channel?

The reader (this function). I've added a defer close(errCh).

---

internal/upload/upload.go, line 148 at r3 (raw file):

Previously, stephen-soltesz (Stephen Soltesz) wrote…

If Run returns on the first error from the readcounterflow goroutine, I would expect the goroutine to quit immediately.

readcounterflow now can only write to the channel once (an error or nil if successful) before returning. At this point readcounterflow has quit already in every case. Is my understanding correct?

[stephen-soltesz]

Reviewable status: complete! 1 of 1 approvals obtained

---

internal/upload/upload.go, line 148 at r3 (raw file):

Previously, robertodauria (Roberto D'Auria) wrote…

readcounterflow now can only write to the channel once (an error or nil if successful) before returning. At this point readcounterflow has quit already in every case. Is my understanding correct?

lgtm 👍