chore: migrate from request to got + fetch #3

Author: easydime

packages/krawler/package.json

@@ -77,6 +77,7 @@
     "feathers-hooks-common": "^3.0.0",
     "fs-blob-store": "^5.2.1",
     "fs-extra": "catalog:",
+    "got": "^14.4.5",
     "helmet": "catalog:",
     "imapflow": "^1.0.162",
     "js-yaml": "^3.13.1",
@@ -88,13 +89,11 @@
     "moment": "catalog:",
     "mongodb": "catalog:",
     "mubsub-es": "^2.0.0",
-    "node-fetch": "^3.2.6",
     "osmtogeojson": "^3.0.0-beta.3",
     "papaparse": "catalog:",
     "pg": "^8.7.3",
     "proj4": "catalog:",
     "reproject": "^1.2.1",
-    "request": "^2.88.0",
     "s3-blob-store": "^2.1.0",
     "sift": "catalog:",
     "socket.io-client": "catalog:",

packages/krawler/src/healthcheck.js

@@ -1,6 +1,4 @@
-import request from 'request'
 import path, { dirname } from 'path'
-import utils from 'util'
 
 import fs from 'fs-extra'
 import _ from 'lodash'
@@ -68,8 +66,8 @@ export async function publishToSlack (slackWebhook, data, compilers, posttext =
     const message = compilers.message(data)
     const link = compilers.link(data)
     const text = link ? `<${link}|${message}${posttext}>` : `${message}${posttext}`
-    await utils.promisify(request.post)({
-      url: slackWebhook,
+    await fetch(slackWebhook, {
+      method: 'POST',
       body: JSON.stringify({
         attachments: [
           {
@@ -106,10 +104,14 @@ export async function healthcheck (options) {
     if (options.debug) {
       logger.info(`Requesting healthcheck endpoint ${endpoint}`)
     }
-    const response = await utils.promisify(request.get)(endpoint)
-    const data = JSON.parse(response.body)
+    const response = await fetch(endpoint)
+    // krawler healthcheck endpoint returns either 200 or 500
+    if (response.status !== 200 && response.status !== 500) {
+      throw new Error('Healthcheck rejected with unexpected HTTP code ' + response.status)
+    }
+    const data = await response.json()
     if (options.debug) {
-      logger.info(`Current healthcheck status from service ${response.statusCode}`)
+      logger.info(`Current healthcheck status from service ${response.status}`)
       logger.info('Current healthcheck output read from service', data)
       logger.info('Previous healthcheck output read from log', previousHealthcheck)
     }

packages/krawler/src/hooks/hooks.auth.js

@@ -1,10 +1,26 @@
 import _ from 'lodash'
-import utils from 'util'
-import request from 'request'
 import makeDebug from 'debug'
 
 const debug = makeDebug('krawler:hooks:auth')
 
+// Apply a tough-cookie-compatible CookieJar (if provided) to fetch headers and persist Set-Cookie back to it.
+// Feature-detects getCookieString/setCookie so non-jar truthy values (e.g. `jar: true`, legacy compat) are tolerated.
+async function fetchWithJar (url, init, jar) {
+  const headers = { ...(init.headers || {}) }
+  if (jar && typeof jar.getCookieString === 'function') {
+    const cookieString = await jar.getCookieString(url)
+    if (cookieString) headers.cookie = cookieString
+  }
+  const response = await fetch(url, { ...init, headers })
+  if (jar && typeof jar.setCookie === 'function') {
+    const setCookies = response.headers.getSetCookie ? response.headers.getSetCookie() : []
+    for (const c of setCookies) {
+      await jar.setCookie(c, url)
+    }
+  }
+  return response
+}
+
 // Add headers for basic/proxy auth
 export function basicAuth (options = {}) {
   return async function (hook) {
@@ -19,7 +35,10 @@ export function basicAuth (options = {}) {
       // Post auth information as form data ?
       if (form) {
         // Set as well if we use cookie to store the session
-        await utils.promisify(request.post)({ url, form, jar: options.jar })
+        await fetchWithJar(url, {
+          method: 'POST',
+          body: new URLSearchParams(form)
+        }, options.jar)
         _.set(requestOptions, 'jar', options.jar)
       } else { // Default method is to directly set basic auth as header
         if (!requestOptions.headers) requestOptions.headers = {}
@@ -49,18 +68,21 @@ export function OAuth (options = {}) {
       let response
       if (!requestOptions.headers) requestOptions.headers = {}
       if (method === 'client_secret_basic') {
-        response = await utils.promisify(request.post)({
-          url,
+        response = await fetch(url, {
+          method: 'POST',
           headers: { Authorization: 'Basic ' + Buffer.from(client_id + ':' + client_secret).toString('base64') },
           body: JSON.stringify(_.omit(oauth, ['client_id', 'client_secret', 'method', 'url']))
         })
       } else if (method === 'client_secret_post') {
-        response = await utils.promisify(request.post)({
-          url,
+        response = await fetch(url, {
+          method: 'POST',
           body: JSON.stringify(_.omit(oauth, ['method', 'url']))
         })
       }
-      const { access_token, token_type } = JSON.parse(response.body)
+      if (!response.ok) {
+        throw new Error('OAuth rejected with HTTP code ' + response.status)
+      }
+      const { access_token, token_type } = await response.json()
       // Defaults to Bearer Auth
       const type = options.type || 'Authorization'
       requestOptions.headers[type] = `${token_type} ${access_token}`

packages/krawler/src/hooks/hooks.feathers.js

@@ -1,7 +1,6 @@
 import _ from 'lodash'
 import feathers from '@feathersjs/client'
 import io from 'socket.io-client'
-import fetch from 'node-fetch'
 import makeDebug from 'debug'
 // import { getItems } from 'feathers-hooks-common'
 import {

packages/krawler/src/hooks/hooks.ogc.js

@@ -1,5 +1,4 @@
 import _ from 'lodash'
-import request from 'request'
 import xml2js from 'xml2js'
 import makeDebug from 'debug'
 
@@ -9,38 +8,29 @@ const debug = makeDebug('krawler:hooks:ogc')
 export function getCapabilities (options = {}) {
   return async function (hook) {
     const queryParameters = _.merge({ request: 'GetCapabilities' }, _.omit(options, ['url', 'headers']))
-    const requestParameters = {
-      method: 'GET',
-      url: options.url,
-      headers: options.headers,
-      qs: queryParameters,
-      qsStringifyOptions: { arrayFormat: 'repeat' }
-    }
 
-    debug('Requesting ' + options.url + ' with following parameters', queryParameters)
-    return new Promise((resolve, reject) => {
-      const onParsed = (err, result) => {
-        if (err) {
-          reject(err instanceof Error ? err : new Error(String(err)))
-          return
-        }
-        // feed the hook with the parsed result
-        _.set(hook, options.dataPath || 'result.data', result)
-        resolve(hook)
+    const url = new URL(options.url)
+    Object.keys(queryParameters).forEach((param) => {
+      const value = queryParameters[param]
+      if (Array.isArray(value)) {
+        value.forEach((val) => url.searchParams.append(param, val))
+      } else {
+        url.searchParams.append(param, value)
       }
-      const onResponse = (error, response, body) => {
-        if (error) {
-          reject(error instanceof Error ? error : new Error(String(error)))
-          return
-        }
-        if (response.statusCode !== 200) {
-          reject(new Error('Request rejected with HTTP code ' + response.statusCode))
-          return
-        }
-        const parser = new xml2js.Parser({ explicitArray: false })
-        parser.parseString(body, onParsed)
-      }
-      request(requestParameters, onResponse)
     })
+
+    debug('Requesting ' + options.url + ' with following parameters', queryParameters)
+    const response = await fetch(url, {
+      method: 'GET',
+      headers: options.headers
+    })
+    if (!response.ok) {
+      throw new Error('Request rejected with HTTP code ' + response.status)
+    }
+    const body = await response.text()
+    const parser = new xml2js.Parser({ explicitArray: false })
+    const result = await parser.parseStringPromise(body)
+    _.set(hook, options.dataPath || 'result.data', result)
+    return hook
   }
 }

packages/krawler/src/services/tasks.js

@@ -63,7 +63,7 @@ class TasksService extends Service {
     return new Promise((resolve, reject) => {
       let statusCode
       taskStream
-        .on('timeout', reject)
+        .on('error', reject)
         .on('response', (response) => {
           statusCode = response.statusCode
           if (response.statusCode > 299) {

packages/krawler/src/tasks/tasks.http.js

@@ -1,30 +1,56 @@
 import _ from 'lodash'
-import request from 'request'
+import got from 'got'
 import makeDebug from 'debug'
 
 const debug = makeDebug('krawler:tasks')
 
+// Build URLSearchParams with array values expanded as repeated keys
+// (replicates request's qsStringifyOptions: { arrayFormat: 'repeat' })
+function buildSearchParams (queryParameters) {
+  const searchParams = new URLSearchParams()
+  for (const key of Object.keys(queryParameters)) {
+    const value = queryParameters[key]
+    if (value === undefined || value === null) continue
+    if (Array.isArray(value)) {
+      value.forEach((v) => searchParams.append(key, v))
+    } else {
+      searchParams.append(key, value)
+    }
+  }
+  return searchParams
+}
+
 // Build the request parameters to download data from input data source
 export function getRequestParameters (options) {
   const queryParameters = _.merge({}, _.omit(options, ['url', 'method', 'body', 'headers', 'timeout', 'auth', 'oauth', 'jar']))
   const requestParameters = {
-    url: options.url,
-    body: options.body,
+    method: options.method || 'GET',
     headers: options.headers,
-    timeout: options.timeout,
-    jar: options.jar,
-    qs: queryParameters,
-    qsStringifyOptions: { arrayFormat: 'repeat' }
+    body: options.body,
+    searchParams: buildSearchParams(queryParameters),
+    cookieJar: options.jar,
+    // Do not throw on non-2xx so the downstream consumer can read statusCode from the 'response' event
+    throwHttpErrors: false
+  }
+  if (options.timeout) {
+    requestParameters.timeout = { request: options.timeout }
   }
-  // Setup request with URL & params
   debug('Requesting with the following parameters', requestParameters)
   return requestParameters
 }
 
+const METHODS_WITH_BODY = new Set(['POST', 'PUT', 'PATCH', 'DELETE'])
+
 // Create the request stream for a task
 export function createRequestStream (options) {
-  const method = options.method || 'GET'
-  return request({ method, ...getRequestParameters(options) })
+  const params = getRequestParameters(options)
+  const stream = got.stream(options.url, params)
+  // got.stream returns a Duplex; for body-bearing methods without an explicit body,
+  // it would hang waiting for written data. Signal end-of-input explicitly.
+  if (METHODS_WITH_BODY.has(params.method) && params.body === undefined) {
+    stream.end()
+  }
+  return stream
 }
 
 export default createRequestStream

packages/krawler/test/cli.test.js

@@ -1,7 +1,6 @@
 import path, { dirname } from 'path'
 import _ from 'lodash'
 import moment from 'moment'
-import request from 'request'
 import utils from 'util'
 import fs from 'fs-extra'
 import mongodb from 'mongodb'
@@ -66,9 +65,10 @@ describe('krawler:cli', () => {
       fs.removeSync(path.join(outputPath, 'RJTT-30-18000-2-1.tif.csv'))
       appServer = await cli(jobfile, { mode: 'setup', api: true, apiPrefix: '/api', port: 3030, messageTemplate: process.env.MESSAGE_TEMPLATE, debug: true, slackWebhook: process.env.SLACK_WEBHOOK_URL })
       // Submit a job to be run
-      const response = await utils.promisify(request.post)({
-        url: 'http://localhost:3030/api/jobs',
-        body: {
+      const response = await fetch('http://localhost:3030/api/jobs', {
+        method: 'POST',
+        headers: { 'content-type': 'application/json' },
+        body: JSON.stringify({
           id: 'job',
           store: 'job-store',
           tasks: [{
@@ -78,10 +78,9 @@ describe('krawler:cli', () => {
               store: 'task-store'
             }
           }]
-        },
-        json: true
+        })
       })
-      const tasks = response.body
+      const tasks = await response.json()
       await appServer.close()
       expect(tasks.length).toBe(1)
       // Check intermediate products have been erased and final product are here
@@ -134,10 +133,9 @@ describe('krawler:cli', () => {
     // Only run as we already setup the app
     await cli(jobfile, { mode: 'runJob', port: 3030, cron: '*/10 * * * * *', run: true, messageTemplate: process.env.MESSAGE_TEMPLATE, debug: false, slackWebhook: process.env.SLACK_WEBHOOK_URL })
     expect(runCount).toBe(1) // First run
-    const response = await utils.promisify(request.get)('http://localhost:3030/healthcheck')
-    // console.log(response.body)
-    expect(response.statusCode).toBe(200)
-    const healthcheck = JSON.parse(response.body)
+    const response = await fetch('http://localhost:3030/healthcheck')
+    expect(response.status).toBe(200)
+    const healthcheck = await response.json()
     // console.log(healthcheck)
     const { error } = await runCommand('node ' + path.join(__dirname, '..', 'healthcheck.js'))
     expect(error).toBeNull()
@@ -162,10 +160,9 @@ describe('krawler:cli', () => {
     await utils.promisify(setTimeout)((1 + remainingSecondsForNextRun) * 1000)
     try {
       expect(runCount).toBeGreaterThanOrEqual(2) // 2 runs
-      const response = await utils.promisify(request.get)('http://localhost:3030/healthcheck')
-      // console.log(response.body)
-      expect(response.statusCode).toBe(500)
-      const healthcheck = JSON.parse(response.body)
+      const response = await fetch('http://localhost:3030/healthcheck')
+      expect(response.status).toBe(500)
+      const healthcheck = await response.json()
       // console.log(healthcheck)
       const { error } = await runCommand('node ' + path.join(__dirname, '..', 'healthcheck.js'))
       expect(error).toBeTruthy()

packages/krawler/test/jobs.cron.test.js

@@ -1,5 +1,4 @@
 import path, { dirname } from 'path'
-import request from 'request'
 import utils from 'util'
 import moment from 'moment'
 import { cli } from '../src/index.js'
@@ -56,10 +55,9 @@ describe('krawler:jobs:cron', () => {
     await utils.promisify(setTimeout)((6 + remainingSecondsForNextRun) * 1000)
     // Check for error with healthcheck
     {
-      const response = await utils.promisify(request.get)('http://localhost:3030/healthcheck')
-      const healthcheck = JSON.parse(response.body)
-      // console.log(healthcheck)
-      expect(response.statusCode).toBe(500)
+      const response = await fetch('http://localhost:3030/healthcheck')
+      const healthcheck = await response.json()
+      expect(response.status).toBe(500)
       expect(healthcheck.isRunning).toBe(true)
       expect(healthcheck.duration).toBeUndefined()
       expect(healthcheck.nbSkippedJobs).toBeGreaterThanOrEqual(1)
@@ -73,10 +71,9 @@ describe('krawler:jobs:cron', () => {
     await utils.promisify(setTimeout)(5000)
     // Now it should have finished
     {
-      const response = await utils.promisify(request.get)('http://localhost:3030/healthcheck')
-      const healthcheck = JSON.parse(response.body)
-      // console.log(healthcheck)
-      expect(response.statusCode).toBe(200)
+      const response = await fetch('http://localhost:3030/healthcheck')
+      const healthcheck = await response.json()
+      expect(response.status).toBe(200)
       expect(healthcheck.isRunning).toBe(false)
       expect(healthcheck.duration).toBeTruthy()
       expect(healthcheck.nbSkippedJobs).toBe(0)