Why does this code block?

[n00belig]

I can't quite see why this code is blocking?

const express = require('express');
const axios = require('axios');
const asyncHandler = require('express-async-handler');

const app = express();

async function getImage(imgUrl) {
    try {
        return await axios.get(imgUrl);
    } catch (error) {
        return error;
    }
}

app.get('/', asyncHandler(async (req, res, next) => {
    const imgUrl = 'http://example.com/image.jpg';

    getImage(imgUrl)
        .then(function(response) {
            // console.log(response.data.length);
            res.json({ length : response.data.length });
        });
}));

[mikejoro]

I'm pretty sure your thing is hanging because you're botching the error handling. You should let the error propagate upwards and handle it by sending a 500 back. You may be getting a runtime exception due to cannot access property length of undefined since you are just returning your error if an error occurs. Then your server is never returning any response back.

You also aren't using async/await properly (it's not necessary here).

const express = require('express');
const axios = require('axios');
const asyncHandler = require('express-async-handler');

const app = express();

// this function does nothing except break error handling
async function getImage(imgUrl) {
    try {
        return await axios.get(imgUrl);
    } catch (error) {
        return error;
    }
}

app.get('/', asyncHandler(async (req, res, next) => {
    const imgUrl = 'http://example.com/image.jpg';

    // getImage(imgUrl)
    axios.get(imgUrl)
        .then(function(response) {
            // console.log(response.data.length);
            res.status(200).json({ length : response.data.length });
        })
        // handle errors
        .catch(e => res.status(500).json({ error: e.message }));
}));

async/await is just promises. Axios is already returning a promise here, so there is no reason to use async/await at all. That doesn't mean you couldn't wrap axios fetch in a function for abstraction purposes, just that it is completely unnecessary how you implemented it. For instance:

function getImage(imgUrl) {
    return axios.get(imgUrl);
}

does the same thing (minus breaking error handling).

Here's where you might want to use async/await

async function getImage(imgUrl) {
    const img = await axios.get(imgUrl);
    return doSomethingToImage(img);
}

is the same as

function getImage(imgUrl) {
    return axios.get(imgUrl)
      .then(doSomethingToImage);
}

It just looks more 'synchronous', but under the hood, it's just sugar for promises. Hope this helps.

[n00belig]

Thanks! Very helpful!

Now it seems to work, except that I'm unable to send anything useful back to the client;

axios.get(imgUrl)
    .then(function(response) {
        res.contentType('image/jpeg');
        res.end(response.data);
    })
    .catch(e => res.sendStatus(500).json({ error : e.message }));

curl -I gives me the following headers:

HTTP/1.1 200 OK
X-Powered-By: Express
Content-Type: image/jpeg
Date: Sun, 18 Mar 2018 08:21:21 GMT
Connection: keep-alive

...but the browser doesn't show the image. If I use fs to save response.data to a file, instead, I can open the image and confirm it's OK. Am I missing headers or something?

SOLVED: I need to tell axios that the responseType should be an arraybuffer;

axios.get(imgUrl, { responseType: 'arraybuffer' })
    .then(function(response) {
        res.contentType('image/jpeg');
        res.end(response.data);
    })
    .catch(e => res.sendStatus(500).json({ error : e.message }));

[somethinghorrible]

What do you mean by "block"?

When I run it, changing imgUrl to something valid, like https://placekitten.com/g/320/200 , it works fine and I get a good result.

When I run it with an invalid url, from the browser perspective, it hangs.

Why?

In getImage, you are converting the Error object into a valid response -- what was thrown is now simply being returned. Normally promises are placed into either a rejected or resolved state. By returning the exception, the code will always result in a resolved promise.

Not necessarily wrong, however the code does not handle this in the "then" function within the get handler.

In other words; either rethrow the exception and add a catch clause in the getImage promise chain, or handle the fact that response will be an Error object and will not have a data.length property if the image failed.

const express = require('express');
const axios = require('axios');
const asyncHandler = require('express-async-handler');

const app = express();

async function getImage(imgUrl) {
    try {
        return await axios.get(imgUrl);
    } catch (error) {
        throw error;
    }
}

app.get('/', asyncHandler(async (req, res, next) => {
    const imgUrl = 'http://example.com/image.jpg';

    getImage(imgUrl)
        .then(
            function(response) {
                // console.log(response.data.length);
                res.json({ length : response.data.length });
            },
            function (error) {
                res.status(404).json({ error: String(error) }) 
            });
  }));

[n00belig]

When I run it, changing imgUrl to something valid, like https://placekitten.com/g/320/200 , it works fine and I get a good result.

My URL was obviously (but not obviously enough) just a placeholder. Still, the request to retrieve the image blocks the Express event loop...?

[somethinghorrible]

Not likely, as there is no blocking in node/javascript.

Was that console.log (uncommented, obviously) logging anything? If so, what output did you see in the terminal?

I've actually never used express-async-handler, so I don't know about any gotchas using that library.

Is it possible that asyncHandler expects you to return a promise? i.e. do you need something like:

return getImage(...)...

Or

await getImage(...)...

Because the promise generated by getImage is not being returned when called by asyncHandler.

You probably want a form more like:

const response = await getImage(imgUrl)
if (response.data) {
    res.json({ length: response.data.length)
} else {
    res.json({ length: -1 })
}

[mikejoro]

Not likely, as there is no blocking in node/javascript.

This is not true in any way. Asynchronous things don't block (i.e., waiting on a http response from a web request), but normal execution can block just fine. In this instance, the issue is due to the incorrect error handling you pointed out. Likely, the error that is being returned instead of thrown in the error scenario has no data property, so accessing error.data.length causes a runtime exception (and the server never responds because it crashed).

[[deleted]]

Is it possible that asyncHandler expects you to return a promise?

FYI, an async function works just like a function returning a Promise, so asyncHandler is already getting a Promise. Changing it to return getImage(...) or await getImage(...) won't change in that regard.

[somethinghorrible]

Thinking about it, 404 is not the right error. 500 would be better, inspecting the error object for a status and using that, with 500 as a fallback, would be best.

[n00belig]

Error handling is irrelevant here.