1

I am trying to download a report that is generated daily on the first request to the report's endpoint.

When the report is being created, the endpoint returns a HTTP 202.

I have the following code to handle some errors and redirects, as well as trying to "sleep" for 60 seconds before continuing try the endpoint again. Unfortunately, the second console log to tell me the download completed is never called, though the file does indeed download successfully and the filestream closes.


// Main function
run()

async function run() {
  await getReport()
  await processReport()
}

async function getReport() {
  console.log(`Downloading ${reportFileName}`)
  await downloadFile(url, reportFileName)
  console.log(`Downloaded ${reportFileName} successfully.`) // This is never called?
}
async function downloadFile (url, targetFile) {  
  return await new Promise((resolve, reject) => {
    https.get(url, async response => {
      const code = response.statusCode ?? 0

      if (code >= 400) {
        return reject(new Error(response.statusMessage))
      }

      // handle redirects
      if (code > 300 && code < 400 && !!response.headers.location) {
        resolve(downloadFile(response.headers.location, targetFile))
        return
      }

      // handle file creation pending
      if (code == 202) {
        console.log(`Report: ${reportFileName} is still being generated, trying again in ${timeToSleepMs/1000} seconds...`)
        await sleep(timeToSleepMs)
        resolve(downloadFile(url, targetFile))
        return
      }

      // make download directory regardless of if it exists
      fs.mkdirSync(outputPath, { recursive: true }, (err) => {
        if (error) throw error;
      });

      // save the file to disk
      const fileWriter = fs
        .createWriteStream(`${outputPath}/${targetFile}`)
        .on('finish', () => {
          resolve({})
        })

      response.pipe(fileWriter)
    }).on('error', error => {
      reject(error)
    })
  })
}

Finally my sleep function:

let timeToSleepMs = (60 * 1000)
function sleep(ms) {
  return new Promise((resolve) => {
    setTimeout(resolve, ms);
  });
}

I'm pretty sure this has to do with some sort of async issue because that always seems to be my issue with Node, but I'm not sure how to handle it. I just want to fetch a file and download it locally, retrying if I get a HTTP 202. If there's a better way, please let me know!

tl;dr - How do I properly handle waiting for a HTTP 202 response to turn into a HTTP 200 when the file is generated, then continue executing code after the file is downloaded?

TomH
  • 117
  • 1
  • 11
  • Can't you end this process and re-run it after a 60 sec wait (using a simple setTimeout() and function call) ? – aymcg31 Nov 14 '22 at 15:44
  • @aycmg31 - I'm not sure what you mean to be honest. I'm not very familiar with async stuff at all unfortunately. This is a script that I want to call once per day, but since the first call to the report endpoint generates the report and that takes 60-180 seconds on average, I was trying to build in a retry without having to call my script a second time all over again. – TomH Nov 14 '22 at 16:03

2 Answers2

2

await downloadFile(url, reportFileName) invokes a recursive function and waits for the promise from the outermost invocation to resolve. But if the function calls itself recursively in

await sleep(timeToSleepMs)
return downloadFile(url, targetFile)

this outermost promise is never resolved.

Replace the two lines above with

await sleep(timeToSleepMs)
resolve(downloadFile(url, targetFile))
return

then the resolution of the outermost promise is the resolution of the recursively invoked second-outermost promise, and so on.

Heiko Theißen
  • 12,807
  • 2
  • 7
  • 31
  • Unfortunately I still have the same issue after making the change you outlined above. That seemed like it would've been the issue, but I'm not sure why it's still not working. Async in JS is the bane of my existence... – TomH Nov 14 '22 at 23:00
  • Please update the question with your most recent code. – Heiko Theißen Nov 15 '22 at 07:02
  • I resolved this. The issue was the first request provides a redirect `HTTP 302`, and in that conditional I was recursively calling like I did for the `HTTP 202` code. So just like your answer, I needed to `resolve` the promise instead of returning with a recursive call and leaving a promise stranded. I updated my question with the working code and marked it resolved. – TomH Nov 15 '22 at 14:26
1

Heiko already identified the problem: when you return from the callback, you never resolve the promise. To avoid such mistakes in general, it is recommended not to mix the promisification of a function and the business logic. Do use async/await only in the latter, do not pass async functions as callbacks. In your case, that would be

function sleep(ms) {
  return new Promise((resolve) => {
    setTimeout(resolve, ms);
  });
}
function httpsGet(url) {
  return new Promise((resolve, reject) => {
    https.get(url, resolve).on('error', reject);
  });
}
function writeFile(path, readableStream) {
  return new Promise((resolve, reject) => {
    const fileWriter = fs
      .createWriteStream(path)
      .on('finish', resolve)
      .on('error', reject);
    readableStream.pipe(fileWriter);
  });
}

Then you can easily write a straightforward function without any callbacks:

async function downloadFile (url, targetFile) {  
  const response = httpsGet(url);
  const code = response.statusCode ?? 0

  if (code >= 400) {
    throw new Error(response.statusMessage);
  }

  // handle redirects
  if (code > 300 && code < 400 && !!response.headers.location) {
    return downloadFile(response.headers.location, targetFile)
  }

  // handle file creation pending
  if (code == 202) {
    console.log(`Report: ${reportFileName} is still being generated, trying again in ${timeToSleepMs/1000} seconds...`)
    await sleep(timeToSleepMs)
    return downloadFile(url, targetFile)
  }

  // make download directory regardless of if it exists
  fs.mkdirSync(outputPath, { recursive: true });

  // save the file to disk
  await writeFile(`${outputPath}/${targetFile}`, response);
}
Bergi
  • 630,263
  • 148
  • 957
  • 1,375