fix(fetch): handle HTML server response better (#928)

__tests__/test-wire.js

@@ -214,8 +214,7 @@ access it.
       expect(error.code).toEqual(E.AssertServerResponseFail)
       expect(error.data).toEqual({
         expected: `Two strings separated by '\\x00'`,
-        actual:
-          `ERR Repository not found
+        actual: `ERR Repository not found
 The requested repository does not exist, or you do not have permission to
 access it.
 `

src/managers/GitRemoteHTTP.js

@@ -1,6 +1,7 @@
 import { E, GitError } from '../models/GitError.js'
 import { calculateBasicAuthHeader } from '../utils/calculateBasicAuthHeader.js'
 import { calculateBasicAuthUsernamePasswordPair } from '../utils/calculateBasicAuthUsernamePasswordPair.js'
+import { collect } from '../utils/collect.js'
 import { extractAuthFromUrl } from '../utils/extractAuthFromUrl.js'
 import { http as builtinHttp } from '../utils/http.js'
 import { pkg } from '../utils/pkg.js'
@@ -102,24 +103,32 @@ export class GitRemoteHTTP {
         statusMessage: res.statusMessage
       })
     }
-    // I'm going to be nice and ignore the content-type requirement unless there is a problem.
-    try {
-      const remoteHTTP = await parseRefsAdResponse(res.body, {
-        service
-      })
+    // Git "smart" HTTP servers should respond with the correct Content-Type header.
+    if (
+      res.headers['content-type'] === `application/x-${service}-advertisement`
+    ) {
+      const remoteHTTP = await parseRefsAdResponse(res.body, { service })
       remoteHTTP.auth = auth
       return remoteHTTP
-    } catch (err) {
-      // Detect "dumb" HTTP protocol responses and throw more specific error message
-      if (
-        err.code === E.AssertServerResponseFail &&
-        err.data.expected === `# service=${service}\\n` &&
-        res.headers['content-type'] !== `application/x-${service}-advertisement`
-      ) {
-        // Ooooooh that's why it failed.
-        throw new GitError(E.RemoteDoesNotSupportSmartHTTP, {})
+    } else {
+      // If they don't send the correct content-type header, that's a good indicator it is either a "dumb" HTTP
+      // server, or the user specified an incorrect remote URL and the response is actually an HTML page.
+      // In this case, we save the response as plain text so we can generate a better error message if needed.
+      const data = await collect(res.body)
+      const response = data.toString('utf8')
+      const preview =
+        response.length < 256 ? response : response.slice(0, 256) + '...'
+      // For backwards compatibility, try to parse it anyway.
+      try {
+        const remoteHTTP = await parseRefsAdResponse([data], { service })
+        remoteHTTP.auth = auth
+        return remoteHTTP
+      } catch (e) {
+        throw new GitError(E.RemoteDoesNotSupportSmartHTTP, {
+          preview,
+          response
+        })
       }
-      throw err
     }
   }
 

src/models/GitError.js

@@ -26,7 +26,7 @@ const messages = {
   RemoteDoesNotSupportDeepenSinceFail: `Remote does not support shallow fetches by date.`,
   RemoteDoesNotSupportDeepenNotFail: `Remote does not support shallow fetches excluding commits reachable by refs.`,
   RemoteDoesNotSupportDeepenRelativeFail: `Remote does not support shallow fetches relative to the current shallow depth.`,
-  RemoteDoesNotSupportSmartHTTP: `Remote does not support the "smart" HTTP protocol, and isomorphic-git does not support the "dumb" HTTP protocol, so they are incompatible.`,
+  RemoteDoesNotSupportSmartHTTP: `Remote did not reply using the "smart" HTTP protocol. Expected "001e# service=git-upload-pack" but received: { preview }`,
   CorruptShallowOidFail: `non-40 character shallow oid: { oid }`,
   FastForwardFail: `A simple fast-forward merge was not possible.`,
   MergeNotSupportedFail: `Merges with conflicts are not supported yet.`,