fixup! tweaks + make tests pass

Also: introduce a new synthetic status code 904 for unsupported host
types (e.g. `ipv4` instead of `ip4`).

Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>

Author: bajtos

deps.ts

@@ -8,7 +8,7 @@ export { encodeHex } from 'https://deno.land/std@0.203.0/encoding/hex.ts'
 export { decodeBase64 } from 'https://deno.land/std@0.203.0/encoding/base64.ts'
 export { decode as decodeVarint } from 'https://deno.land/x/varint@v2.0.0/varint.ts'
 
-// Deno Bundle does not support npm dependencies, we have to load the via CDN
+// Deno Bundle does not support npm dependencies, we have to load them via CDN
 export { CarBlockIterator } from 'https://cdn.skypack.dev/@ipld/car@5.3.2/?dts'
 export {
   UnsupportedHashError,

lib/multiaddr.js

@@ -26,7 +26,7 @@ export function multiaddrToHttpUri (addr) {
     )
   }
 
-  return `${scheme}://${getUriHost(hostType, hostValue)}${buildUriPort(scheme, port)}`
+  return `${scheme}://${getUriHost(hostType, hostValue)}${getUriPort(scheme, port)}`
 }
 
 function getUriHost (hostType, hostValue) {
@@ -37,11 +37,18 @@ function getUriHost (hostType, hostValue) {
     case 'dns6':
       return hostValue
     case 'ip6':
+      // See https://superuser.com/a/367788/135774:
+      // According to RFC2732, literal IPv6 addresses should be put inside square brackets in URLs
       return `[${hostValue}]`
   }
+
+  throw Object.assign(
+    new Error(`Unsupported multiaddr host type "${hostType}"`),
+    { code: 'UNSUPPORTED_MULTIADDR_HOST_TYPE' }
+  )
 }
 
-function buildUriPort (scheme, port) {
+function getUriPort (scheme, port) {
   if (scheme === 'http' && port === '80') return ''
   if (scheme === 'https' && port === '443') return ''
   return `:${port}`

lib/spark.js

@@ -88,6 +88,9 @@ export default class Spark {
     } catch (err) {
       console.error(`Failed to fetch ${retrieval.cid} from ${provider.address} using ${provider.protocol}`)
       console.error(err)
+      if (!stats.statusCode) {
+        stats.statusCode = mapErrorToStatusCode(err)
+      }
     }
   }
 
@@ -145,13 +148,7 @@ export default class Spark {
         }
 
         if (!stats.carTooLarge) {
-          try {
-            await verifyContent(cid, carBytes)
-            stats.contentVerification = 'OK'
-          } catch (err) {
-            console.error('Content verification failed.', err)
-            stats.contentVerification = 'ERROR_' + (err.code ?? 'UNKNOWN')
-          }
+          await verifyContent(cid, carBytes)
 
           const digest = await crypto.subtle.digest('sha-256', carBytes)
           // 12 is the code for sha2-256
@@ -162,11 +159,6 @@ export default class Spark {
         console.error('Retrieval failed with status code %s: %s',
           res.status, (await res.text()).trimEnd())
       }
-    } catch (err) {
-      if (!stats.statusCode) {
-        stats.statusCode = mapErrorToStatusCode(err)
-      }
-      throw err
     } finally {
       clearTimeout(timeout)
     }
@@ -273,7 +265,13 @@ function getRetrievalUrl (protocol, address, cid) {
  * @param {Uint8Array} carBytes
  */
 async function verifyContent (cid, carBytes) {
-  const reader = await CarBlockIterator.fromBytes(carBytes)
+  let reader
+  try {
+    reader = await CarBlockIterator.fromBytes(carBytes)
+  } catch (err) {
+    throw Object.assign(err, {code: 'CANNOT_PARSE_CAR_BYTES' })
+  }
+
   for await (const block of reader) {
     if (block.cid.toString() !== cid.toString()) {
       throw Object.assign(
@@ -295,6 +293,8 @@ function mapErrorToStatusCode (err) {
       return 902
     case 'MULTIADDR_HAS_TOO_MANY_PARTS':
       return 903
+    case 'UNSUPPORTED_MULTIADDR_HOST_TYPE':
+      return 904
   }
 
   // 92x for content verification errors
@@ -304,6 +304,8 @@ function mapErrorToStatusCode (err) {
     return 922
   } else if (err.code === 'UNEXPECTED_CAR_BLOCK') {
     return 923
+  } else if (err.code === 'CANNOT_PARSE_CAR_BYTES') {
+    return 924
   }
 
   // 91x errors for network connection errors

test/spark.js

@@ -5,6 +5,8 @@ import { test } from 'zinnia:test'
 import { assertInstanceOf, assertEquals, assertArrayIncludes } from 'zinnia:assert'
 import { SPARK_VERSION } from '../lib/constants.js'
 
+const KNOWN_CID = 'bafkreih25dih6ug3xtj73vswccw423b56ilrwmnos4cbwhrceudopdp5sq'
+
 test('getRetrieval', async () => {
   const round = {
     roundId: '123',
@@ -47,19 +49,12 @@ test('getRetrieval', async () => {
 
 // TODO: test more cases
 test('fetchCAR', async () => {
-  const URL = 'url'
   const requests = []
-  const fetch = async url => {
-    requests.push({ url })
-    return {
-      status: 200,
-      ok: true,
-      body: (async function * () {
-        yield new Uint8Array([1, 2, 3])
-      })()
-    }
+  const mockedFetch = async url => {
+    requests.push(url.toString())
+    return fetch(`https://frisbii.fly.dev/ipfs/${KNOWN_CID}`)
   }
-  const spark = new Spark({ fetch })
+  const spark = new Spark({ fetch: mockedFetch })
   const stats = {
     timeout: false,
     startAt: new Date(),
@@ -70,16 +65,16 @@ test('fetchCAR', async () => {
     carChecksum: null,
     statusCode: null
   }
-  await spark.fetchCAR('http', '127.0.0.1', 'bafy', stats)
-  assertEquals(stats.timeout, false)
+  await spark.fetchCAR('http', '/ip4/127.0.0.1/tcp/80/http', KNOWN_CID, stats)
+  assertEquals(stats.timeout, false, 'stats.timeout')
   assertInstanceOf(stats.startAt, Date)
   assertInstanceOf(stats.firstByteAt, Date)
   assertInstanceOf(stats.endAt, Date)
-  assertEquals(stats.carTooLarge, false)
-  assertEquals(stats.byteLength, 3)
-  assertEquals(stats.carChecksum, '1220039058c6f2c0cb492c533b0a4d14ef77cc0f78abccced5287d84a1a2011cfb81')
-  assertEquals(stats.statusCode, 200)
-  assertEquals(requests, [{ url: URL }])
+  assertEquals(stats.carTooLarge, false, 'stats.carTooLarge')
+  assertEquals(stats.byteLength, 200, 'stats.byteLength')
+  assertEquals(stats.carChecksum, '122069f03061f7ad4c14a5691b7e96d3ddd109023a6539a0b4230ea3dc92050e7136', 'stats.carChecksum')
+  assertEquals(stats.statusCode, 200, 'stats.statusCode')
+  assertEquals(requests, [`http://127.0.0.1/ipfs/${KNOWN_CID}?dag-scope=block`])
 })
 
 /* Disabled as long as we are fetching the top-level block only
@@ -104,7 +99,7 @@ test('fetchCAR exceeding MAX_CAR_SIZE', async () => {
     carChecksum: null,
     statusCode: null
   }
-  await spark.fetchCAR('http', '127.0.0.1', 'bafy', stats)
+  await spark.fetchCAR('http', '/ipv4/127.0.0.1/tcp/80/http', 'bafy', stats)
   assertEquals(stats.timeout, false)
   assertEquals(stats.carTooLarge, true)
   assertEquals(stats.byteLength, MAX_CAR_SIZE + 1)