A functional programmer would say you’re going wrong by modifying external state from inside a function. Another way of looking at it would say that you’re going wrong by falling for the fiction that async and await are magical time machines that make asynchronous things happen synchronously.
Personally, I don’t use or recommend async and await for precisely this reason. They obscure what actually is happening and present a facade that tempts us to write things like you have here.
The bottom line is that nobody outside test has any clue when the token has been received from storage. Yet, you rely on it having been set from inside getDeployments. Result: race condition.
So here’s what I would recommend: get rid of async and await, and instead provide proper return types for every function you write. Follow the advice in taxonomy of asynchronous beasts and make the first word of every class C asynchronous function you write be return.
This situation is exactly why I give both sides of that advice: it forces us to think what test should return (some sort of future), and locks us into returning the very beginning of the chain of events, which is going to start with the Promise returned from Storage.get.
So you would end up with something structured like this (renaming test to something more descriptive of what it’s actually doing):
fetchToken(): Promise<string> {
return Storage.get({key: "token-key"});
}
getDeployments(): Observable<Deployment[]> {
return from(this.fetchToken()).pipe(
switchMap(token => this.http.get(`${this.env.API_URL}/api/deployments`, { headers: { 'Authorization': `Bearer ${token}`));
}
In the real world, you don’t likely want to be hitting Storage every time here, so you would make your token property a Promise or a Subject, feed it at app startup, and reference it from inside getDeployments. Don’t make it a naked value, though, like you have here, because it’s then impossible for consumers to know when they can reliably access it, as you have discovered the hard way.