Repository: syncthing/syncthing (language: go)

Issue:
Periodic connection losses to webui because docker image doesn't support upgrade
### Environment
I am using the official syncthing/syncthing docker image in the latest version (v1.21.0).

### Problem
Every now and then the webui displays for a short moment <1s that the connection to the server is lost.

The reason is known. It is because the webui fails to call:
https://mybaseurl/rest/system/upgrade

This endpoint responds http status code 500 and the following text:
`upgrade unsupported`
(which is normal for the docker image)

The webui doesn't seem to understand the response and interprets it as server failure.

### Proposed changes
- Change the responded http status code to "501 Not Implemented"
- Accept code 501 as valid response for the call to the upgrade-check endpoint so that the webui doesn't display connection errors.





Discussion / hints:
Interesting, I can see how this could happen, but I've never actually seen it in real life and I talk to Dockerized Syncthings all the time. I guess it may be timing related, as the upgrade check is done on startup (of the GUI) and a lot of other requests happen at the same time.

Nonetheless I agree that returning not-implemented is better and could be specially handled in the GUI!

Fix patch:
diff --git a/gui/default/syncthing/core/syncthingController.js b/gui/default/syncthing/core/syncthingController.js
index 49d9d446e87..1280145f595 100755
--- a/gui/default/syncthing/core/syncthingController.js
+++ b/gui/default/syncthing/core/syncthingController.js
@@ -183,8 +183,9 @@ angular.module('syncthing.core')
                 if (arg.status === 0) {
                     // A network error, not an HTTP error
                     $scope.$emit(Events.OFFLINE);
-                } else if (arg.status >= 400 && arg.status <= 599) {
-                    // A genuine HTTP error
+                } else if (arg.status >= 400 && arg.status <= 599 && arg.status != 501) {
+                    // A genuine HTTP error. 501/NotImplemented is considered intentional 
+                    // and not an error which we need to act upon.
                     $('#networkError').modal('hide');
                     $('#restarting').modal('hide');
                     $('#shutdown').modal('hide');
diff --git a/lib/api/api.go b/lib/api/api.go
index f24a974112d..5ca8f521528 100644
--- a/lib/api/api.go
+++ b/lib/api/api.go
@@ -864,7 +864,7 @@ func (s *service) getDBRemoteNeed(w http.ResponseWriter, r *http.Request) {
 	device := qs.Get("device")
 	deviceID, err := protocol.DeviceIDFromString(device)
 	if err != nil {
-		http.Error(w, err.Error(), 500)
+		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
 
@@ -1017,7 +1017,7 @@ func (s *service) postSystemReset(w http.ResponseWriter, r *http.Request) {
 
 	if len(folder) > 0 {
 		if _, ok := s.cfg.Folders()[folder]; !ok {
-			http.Error(w, "Invalid folder ID", 500)
+			http.Error(w, "Invalid folder ID", http.StatusInternalServerError)
 			return
 		}
 	}
@@ -1291,7 +1291,7 @@ func (s *service) getReport(w http.ResponseWriter, r *http.Request) {
 		version = val
 	}
 	if r, err := s.urService.ReportDataPreview(context.TODO(), version); err != nil {
-		http.Error(w, err.Error(), 500)
+		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	} else {
 		sendJSON(w, r)
@@ -1316,7 +1316,7 @@ func (s *service) getDBIgnores(w http.ResponseWriter, r *http.Request) {
 
 	lines, patterns, err := s.model.LoadIgnores(folder)
 	if err != nil && !ignore.IsParseError(err) {
-		http.Error(w, err.Error(), 500)
+		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
 
@@ -1333,20 +1333,20 @@ func (s *service) postDBIgnores(w http.ResponseWriter, r *http.Request) {
 	bs, err := io.ReadAll(r.Body)
 	r.Body.Close()
 	if err != nil {
-		http.Error(w, err.Error(), 500)
+		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
 
 	var data map[string][]string
 	err = json.Unmarshal(bs, &data)
 	if err != nil {
-		http.Error(w, err.Error(), 500)
+		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
 
 	err = s.model.SetIgnores(qs.Get("folder"), data["ignore"])
 	if err != nil {
-		http.Error(w, err.Error(), 500)
+		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
 
@@ -1424,13 +1424,13 @@ func (s *service) getEventSub(mask events.EventType) events.BufferedSubscription
 
 func (s *service) getSystemUpgrade(w http.ResponseWriter, _ *http.Request) {
 	if s.noUpgrade {
-		http.Error(w, upgrade.ErrUpgradeUnsupported.Error(), http.StatusServiceUnavailable)
+		http.Error(w, upgrade.ErrUpgradeUnsupported.Error(), http.StatusNotImplemented)
 		return
 	}
 	opts := s.cfg.Options()
 	rel, err := upgrade.LatestRelease(opts.ReleasesURL, build.Version, opts.UpgradeToPreReleases)
 	if err != nil {
-		http.Error(w, err.Error(), 500)
+		httpError(w, err)
 		return
 	}
 	res := make(map[string]interface{})
@@ -1472,8 +1472,7 @@ func (s *service) postSystemUpgrade(w http.ResponseWriter, _ *http.Request) {
 	opts := s.cfg.Options()
 	rel, err := upgrade.LatestRelease(opts.ReleasesURL, build.Version, opts.UpgradeToPreReleases)
 	if err != nil {
-		l.Warnln("getting latest release:", err)
-		http.Error(w, err.Error(), 500)
+		httpError(w, err)
 		return
 	}
 
@@ -1481,7 +1480,7 @@ func (s *service) postSystemUpgrade(w http.ResponseWriter, _ *http.Request) {
 		err = upgrade.To(rel)
 		if err != nil {
 			l.Warnln("upgrading:", err)
-			http.Error(w, err.Error(), 500)
+			http.Error(w, err.Error(), http.StatusInternalServerError)
 			return
 		}
 
@@ -1528,7 +1527,7 @@ func (s *service) makeDevicePauseHandler(paused bool) http.HandlerFunc {
 		if msg != "" {
 			http.Error(w, msg, status)
 		} else if err != nil {
-			http.Error(w, err.Error(), 500)
+			http.Error(w, err.Error(), http.StatusInternalServerError)
 		}
 	}
 }
@@ -1540,7 +1539,7 @@ func (s *service) postDBScan(w http.ResponseWriter, r *http.Request) {
 		subs := qs["sub"]
 		err := s.model.ScanFolderSubdirs(folder, subs)
 		if err != nil {
-			http.Error(w, err.Error(), 500)
+			http.Error(w, err.Error(), http.StatusInternalServerError)
 			return
 		}
 		nextStr := qs.Get("next")
@@ -1551,7 +1550,7 @@ func (s *service) postDBScan(w http.ResponseWriter, r *http.Request) {
 	} else {
 		errors := s.model.ScanFolders()
 		if len(errors) > 0 {
-			http.Error(w, "Error scanning folders", 500)
+			http.Error(w, "Error scanning folders", http.StatusInternalServerError)
 			sendJSON(w, errors)
 			return
 		}
@@ -1571,7 +1570,7 @@ func (*service) getQR(w http.ResponseWriter, r *http.Request) {
 	var text = qs.Get("text")
 	code, err := qr.Encode(text, qr.M)
 	if err != nil {
-		http.Error(w, "Invalid", 500)
+		http.Error(w, "Invalid", http.StatusInternalServerError)
 		return
 	}
 
@@ -1612,7 +1611,7 @@ func (s *service) getFolderVersions(w http.ResponseWriter, r *http.Request) {
 	qs := r.URL.Query()
 	versions, err := s.model.GetFolderVersions(qs.Get("folder"))
 	if err != nil {
-		http.Error(w, err.Error(), 500)
+		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
 	sendJSON(w, versions)
@@ -1624,20 +1623,20 @@ func (s *service) postFolderVersionsRestore(w http.ResponseWriter, r *http.Reque
 	bs, err := io.ReadAll(r.Body)
 	r.Body.Close()
 	if err != nil {
-		http.Error(w, err.Error(), 500)
+		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
 
 	var versions map[string]time.Time
 	err = json.Unmarshal(bs, &versions)
 	if err != nil {
-		http.Error(w, err.Error(), 500)
+		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
 
 	ferr, err := s.model.RestoreFolderVersions(qs.Get("folder"), versions)
 	if err != nil {
-		http.Error(w, err.Error(), 500)
+		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
 	sendJSON(w, errorStringMap(ferr))
@@ -2015,3 +2014,11 @@ func isFolderNotFound(err error) bool {
 	}
 	return false
 }
+
+func httpError(w http.ResponseWriter, err error) {
+	if errors.Is(err, upgrade.ErrUpgradeUnsupported) {
+		http.Error(w, upgrade.ErrUpgradeUnsupported.Error(), http.StatusNotImplemented)
+	} else {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+	}
+}


Test patch: