feat: replace node-fetch with builtin (#834)

Author: ascorbic

src/helpers/functions.js

@@ -22,6 +22,10 @@ exports.generateFunctions = async (
     await ensureDir(join(functionsDir, func))
     await writeFile(join(functionsDir, func, `${func}.js`), handlerSource)
     await copyFile(bridgeFile, join(functionsDir, func, 'bridge.js'))
+    await copyFile(
+      join(__dirname, '..', '..', 'lib', 'templates', 'handlerUtils.js'),
+      join(functionsDir, func, 'handlerUtils.js'),
+    )
   }
 
   await writeHandler(HANDLER_FUNCTION_NAME, false)

src/templates/getHandler.js

@@ -1,13 +1,11 @@
-/* eslint-disable max-lines-per-function */
-const { promises, createWriteStream, existsSync } = require('fs')
+const { promises, existsSync } = require('fs')
 const { Server } = require('http')
 const { tmpdir } = require('os')
 const path = require('path')
-const { promisify } = require('util')
 
-const streamPipeline = promisify(require('stream').pipeline)
 const { Bridge } = require('@vercel/node/dist/bridge')
-const fetch = require('node-fetch')
+
+const { downloadFile } = require('./handlerUtils')
 
 const makeHandler =
   () =>
@@ -55,14 +53,7 @@ const makeHandler =
 
               // Append the path to our host and we can load it like a regular page
               const url = `${base}/${filePath}`
-              console.log(`Downloading ${url} to ${cacheFile}`)
-              const response = await fetch(url)
-              if (!response.ok) {
-                // Next catches this and returns it as a not found file
-                throw new Error(`Failed to fetch ${url}`)
-              }
-              // Stream it to disk
-              await streamPipeline(response.body, createWriteStream(cacheFile))
+              await downloadFile(url, cacheFile)
             }
             // Return the cache file
             return readfileOrig(cacheFile, options)
@@ -163,12 +154,10 @@ const makeHandler =
 const getHandler = ({ isODB = false, publishDir = '../../../.next', appDir = '../../..' }) => `
 const { Server } = require("http");
 const { tmpdir } = require('os')
-const { promises, createWriteStream, existsSync } = require("fs");
-const { promisify } = require('util')
-const streamPipeline = promisify(require('stream').pipeline)
+const { promises, existsSync } = require("fs");
 // We copy the file here rather than requiring from the node module
 const { Bridge } = require("./bridge");
-const fetch = require('node-fetch')
+const { downloadFile } = require('./handlerUtils')
 
 const { builder } = require("@netlify/functions");
 const { config }  = require("${publishDir}/required-server-files.json")
@@ -186,4 +175,3 @@ exports.handler = ${
 `
 
 module.exports = getHandler
-/* eslint-enable max-lines-per-function */

src/templates/handlerUtils.ts

@@ -0,0 +1,33 @@
+import { createWriteStream } from 'fs'
+import http from 'http'
+import https from 'https'
+import { pipeline } from 'stream'
+import { promisify } from 'util'
+
+const streamPipeline = promisify(pipeline)
+
+export const downloadFile = async (url, destination) => {
+  console.log(`Downloading ${url} to ${destination}`)
+
+  const httpx = url.startsWith('https') ? https : http
+
+  await new Promise((resolve, reject) => {
+    const req = httpx.get(url, { timeout: 10000 }, (response) => {
+      if (response.statusCode < 200 || response.statusCode > 299) {
+        reject(new Error(`Failed to download ${url}: ${response.statusCode} ${response.statusMessage || ''}`))
+        return
+      }
+      const fileStream = createWriteStream(destination)
+      streamPipeline(response, fileStream)
+        .then(resolve)
+        .catch((error) => {
+          console.log(`Error downloading ${url}`, error)
+          reject(error)
+        })
+    })
+    req.on('error', (error) => {
+      console.log(`Error downloading ${url}`, error)
+      reject(error)
+    })
+  })
+}

test/index.js

@@ -4,6 +4,7 @@ const process = require('process')
 const os = require('os')
 const cpy = require('cpy')
 const { dir: getTmpDir } = require('tmp-promise')
+const { downloadFile } = require('../src/templates/handlerUtils')
 
 const plugin = require('../src')
 
@@ -186,8 +187,10 @@ describe('onBuild()', () => {
 
     expect(existsSync(`.netlify/internal-functions/___netlify-handler/___netlify-handler.js`)).toBeTruthy()
     expect(existsSync(`.netlify/internal-functions/___netlify-handler/bridge.js`)).toBeTruthy()
+    expect(existsSync(`.netlify/internal-functions/___netlify-handler/handlerUtils.js`)).toBeTruthy()
     expect(existsSync(`.netlify/internal-functions/___netlify-odb-handler/___netlify-odb-handler.js`)).toBeTruthy()
     expect(existsSync(`.netlify/internal-functions/___netlify-odb-handler/bridge.js`)).toBeTruthy()
+    expect(existsSync(`.netlify/internal-functions/___netlify-odb-handler/handlerUtils.js`)).toBeTruthy()
   })
 
   test('writes correct redirects to netlifyConfig', async () => {
@@ -448,3 +451,37 @@ describe('utility functions', () => {
     }
   })
 })
+
+describe('function helpers', () => {
+  it('downloadFile can download a file', async () => {
+    const url =
+      'https://raw.githubusercontent.com/netlify/netlify-plugin-nextjs/c2668af24a78eb69b33222913f44c1900a3bce23/manifest.yml'
+    const tmpFile = join(os.tmpdir(), 'next-test', 'downloadfile.txt')
+    await ensureDir(path.dirname(tmpFile))
+    await downloadFile(url, tmpFile)
+    expect(existsSync(tmpFile)).toBeTruthy()
+    expect(readFileSync(tmpFile, 'utf8')).toMatchInlineSnapshot(`
+      "name: netlify-plugin-nextjs-experimental
+      "
+    `)
+    await unlink(tmpFile)
+  })
+
+  it('downloadFile throws on bad domain', async () => {
+    const url = 'https://nonexistentdomain.example'
+    const tmpFile = join(os.tmpdir(), 'next-test', 'downloadfile.txt')
+    await ensureDir(path.dirname(tmpFile))
+    await expect(downloadFile(url, tmpFile)).rejects.toThrowErrorMatchingInlineSnapshot(
+      `"getaddrinfo ENOTFOUND nonexistentdomain.example"`,
+    )
+  })
+
+  it('downloadFile throws on 404', async () => {
+    const url = 'https://example.com/nonexistentfile'
+    const tmpFile = join(os.tmpdir(), 'next-test', 'downloadfile.txt')
+    await ensureDir(path.dirname(tmpFile))
+    await expect(downloadFile(url, tmpFile)).rejects.toThrowError(
+      'Failed to download https://example.com/nonexistentfile: 404 Not Found',
+    )
+  })
+})