How do I defer the execution of code until fetch r

2019-08-22 21:38发布

问题:

I created the following two functions to get a JWT Token from my backend and store it in localStorage. The logic seems pretty simple but it's not working because before the value of my token is resolved, the code continues executing. As a result, functions in the chain that need the token don't get it. Ultimately, it does get the token and stores it in localStorage but by that time all my functions in the start up chain fail to get a valid token.

I'm doing all of this at the very beginning as my React app is starting up. So the idea is to get the token and then execute the remaining start up steps.

So, I'm calling myStartup() function at the entry point to my app. I do this as the first step. From there, the remaining start up functions are passed as a callback.

export const getJwtTokenFromApi = () => {

    var request = new Request('/api/token', {
        method: 'GET',
        mode: 'cors',
        credentials: 'include'
    });

    fetch(request)
        .then((response) => {
            response.text()
            .then((token) => {
                if(token.length > 0) {
                    localStorage.setItem('myToken', token);
                    return token;
                } else {
                    return null;
                }
             })
        })
        .catch(err => {
        });
}

export const getJwtToken = () => {

    let token = localStorage.getItem('myToken');

    if (token == null)
        token = getJwtTokenFromApi();

    return token;
}

export const myStartup = (callback) => {

    const token = getJwtToken();
    callback(token);
}

What ends up happeningis that the calling function that needs the token is not deferred until the token is received so it ends up receiving undefined.

How do I make sure I defer the execution of my function that needs the token until I have a legit token -- or at least a null value which would mean my API call failed?

回答1:

The fundamental problem is that you're not handling all the async stuff as though it's async. This is a reasonably complicated workflow, with blocking and non-blocking tasks intermixed, which means that we need to apply asynchronous patterns throughout. Let's step through the script one function at a time.


This appears to be the script's entry point:

export const myStartup = (callback) => {
    const token = getJwtToken();
    callback(token);
}

It won't work, because getJwtToken is async, which means that its value will not be available to callback on the next line.

How do we know that getJwtToken is async? Because getJwtToken invokes getJwtTokenFromApi, which invokes fetch (which the spec tells us is async). Since getJwtToken wraps async behavior, it is itself async.

Since getJwtToken is async, we know that token is not going to be available on the second line when callback needs it. In fact, token will never be available in that scope, because getJwtToken returns a Promise, whose resolution value will only be available inside a .then handler. So, step 1 is to rewrite this function:

export const myStartup = (callback) => {
    getJwtToken() // returns a Promise
    .then((token) => { // token is available inside Promise's .then
        callback(token);
    })
}

Now we look inside getJwtToken, bearing in mind that it must return a Promise because of the changes we just made.

export const getJwtToken = () => {
    let token = localStorage.getItem('myToken');
    if (token == null)
        token = getJwtTokenFromApi();
    return token;
}

This is an interesting case, because getJwtToken implements branching behavior, one branch of which is synchronous, and the other not. (localStorage.getItem blocks, but getJwtTokenFromApi is async.) The only way to handle cases like this is to make the entire function async: to make sure that it always returns a Promise, even if the data it needs is available from a sync source.

Since localStorage.getItem is synchronous, if we like the value it gives us, we wrap that value in a Promise before returning it. Otherwise, we can just return the Promise returned by getJwtTokenFromApi:

export const getJwtToken = () => {
    let token = localStorage.getItem('myToken')

    if(token !== null)
        return Promise.resolve(token);

    return getJwtTokenFromApi();
}

Now, no matter which scenario we find ourselves in, this function will return a Promise that contains a token.

Finally, we get to getJwtTokenFromApi, which does a few things:

  • it constructs a Request
  • it executes a request (async)
  • if successful, it converts the response to text (async)
  • it inspects the text

If all those things work out, it wants to return the text value. But half of those tasks are async, which again means that the entire function must become async. Here's a slimmer version of what you started with:

export const getJwtTokenFromApi = () => {

    var request = new Request('/api/token', {});

    fetch(request)
    .then((response) => {
        response.text()
        .then((token) => {
            if(token.length > 0) {
                localStorage.setItem('myToken', token);
                return token;
            } else {
                return null;
            }
         })
    })
}

The biggest problem here is that you're not returning the fetch. This is important, because the other return statements nested inside don't apply to the overall function. This function will not return anything as written, although it will perform an XHR call. So, the first fix is to return fetch.

But just adding that return isn't enough. Why? Because within the .then handler, you want to access the text of the response, but that access is itself async. While you are using a .then to access the value (as token), that value will die silently inside fetch.then unless you also return response.text(). Really, what you need is this:

return fetch(request)
.then((response) => {
    return response.text()
    .then((text) => {
        if(text.length > 0) return text;
        else return null

But this code is needlessly verbose, and the way it creeps to the right with deeper and deeper nesting makes for code that is hard to read or re-order. These steps are sequential, and we want them to look that way:

STEP 1
STEP 2
STEP 3

(not)
STEP 1
    STEP 2
        STEP 3

So, let's try something slimmer:

return fetch(request)                          // step 1
.then((response) => response.text())           // step 2
.then((text) => text.length > 0 ? text : null) // step 3

This code is flatter and slimmer. It's also easier to re-order the steps or insert new ones. Of course, it doesn't do the important work of storing the token in localStorage, which is why we have the slightly beefier final version:

export const getJwtTokenFromApi = () => {

    var request = new Request('/api/token', {
        method: 'GET',
        mode: 'cors',
        credentials: 'include'
    });

    return fetch(request)
    .then((response) => response.text())
    .then((token) => {
        if(token.length > 0) {
            localStorage.setItem('myToken', token);
            return token;
        }

        return null;
     })
    })
}

We're able to flatten all this code because of the way nested Promises resolve: when one Promise contains another Promise (and another, etc.), the engine will automatically unwrap all of the intermediate promises. As an example, these two snippets produce identical results:

var x = Promise.resolve( Promise.resolve( Promise.resolve ( 10 )))
var y = Promise.resolve( 10 )

Both x and y will act like single, flat Promises that resolve to 10, which means we can put this after either one:

.then((value) => {
    // value === 10
})

Here's the final script:

export const getJwtTokenFromApi = () => {

    var request = new Request('/api/token', {
        method: 'GET',
        mode: 'cors',
        credentials: 'include'
    });

    return fetch(request)
    .then((response) => response.text())
    .then((token) => {
        if(token.length > 0) {
            localStorage.setItem('myToken', token);
            return token;
        }

        return null;
     })
    })
}

export const getJwtToken = () => {
    let token = localStorage.getItem('myToken')

    if(token !== null)
        return Promise.resolve(token);

    return getJwtTokenFromApi();
}

export const myStartup = (callback) => {
    getJwtToken()
    .then((token) => {
        callback(token);
    })
}

One more question: is myStartup async or not?

Using the rule of thumb from above, we'd say that since it wraps async behavior, it is itself async. However, this script mixes async patterns: both Promises & callbacks. I suspect this is because you are more familiar with node-style callbacks one the one hand, but fetch returns a Promise, and during implementation those two approaches kind of "meet in the middle" -- or rather, at the module's API: myStartup. It's an async function, but it doesn't seem comfortable with that fact.

When a caller invokes myStartup, it will return nothing. That much is obvious because there is no return statement. But, by accepting a callback function, you've provided a mechanism to signal callers once all the potentially-async work is complete, which means it can still be used.

Unless it's important to support the node-style callback pattern, I'd recommend taking the final step to make this module thoroughly Promise-based: modify myStartup so that it returns a Promise that resolves with the token. Because of the aforementioned unwrapping behavior, this is an extremely simple change:

export const myStartup = () => {
    return getJwtToken();
}

But now it's obvious that myStartup adds nothing to the process, so you might as well remove the wrapper by deleting the function and renaming getJwtToken to myStartup.



回答2:

Your function getJwtToken should return the promise:

export const getJwtToken = () => {   
  let token = localStorage.getItem('myToken');
  return token ? Promise.resolve(token) : getJwtTokenFromApi(storeToken) 
}

in your caller, the token will be wrapped in the inside returned promise:

getJwtToken().then(token => doSomething(token))


回答3:

I got this working with the following code but I don't think it's very elegant.

Essentially, I combined multiple functions into one and added a callback parameter so that I can create a chain of start up functions. If no callback is received, I simply return the token in order to make the getJwtToken() a multi purpose function i.e. either call it to get the token or pass a function that expects the token.

I really would like to have separate functions so that not all concerns are in one function. Also, not crazy about having callback parameter for those times when I need to just get the token.

I wanted to post the code so that I can get some suggestions to make it more robust and elegant.

export const getJwtToken = (callback) => {

    // If token is already in localStorage, get it and return it.
    const token = localStorage.getItem('myToken');
    if (token != null)
        return token;

    // Token is not in localStorage. Get it from API
    var request = new Request('/api/token', {
        method: 'GET',
        mode: 'cors',
        credentials: 'include'
    });

    fetch(request)
        .then((response) => {

            response.text()
                .then((token) => {

                    if (token.length > 0) {

                        // First, save it in localStorage
                        localStorage.setItem('myToken', token);

                        // If no callback function received, just return token
                        if (typeof callback == "undefined") {

                            return token;
                        } else {

                            callback(token);
                        }
                    }
                })
        })
        .catch(err => {
        });
}