Skip to content

Recursive fetch doesn't work on server-side #5168

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
rosieks opened this issue Apr 27, 2017 · 8 comments
Closed

Recursive fetch doesn't work on server-side #5168

rosieks opened this issue Apr 27, 2017 · 8 comments
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-spa

Comments

@rosieks
Copy link
Contributor

rosieks commented Apr 27, 2017

I tried to run such code on server-side but for some reason it doesn't work. fetch function is imported from domain-task

return fetch('https://jsonplaceholder.typicode.com/posts/1')
    .then(response => response.json())
    .then(response => {
        return fetch('https://jsonplaceholder.typicode.com/posts/2')
            .then(response2 => response2.json())
    });

I receive the following error:

Exception: Call to Node module failed with error: Error: 
When running outside the browser (e.g., in Node.js), you must specify a base URL
before invoking domain-task's 'fetch' wrapper.
Example:
import { baseUrl } from 'domain-task/fetch';
baseUrl('http://example.com'); // Relative URLs will be resolved against this
@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented May 2, 2017

If you want server-side prerendering to wait for your chain of promises to complete, then you must add the outer promise to the list of domain tasks, like the WeatherForecasts.ts example does in the ReactRedux template.

For example,

import { fetch, addTask } from 'domain-task';

... and then...

const myTask = fetch('https://jsonplaceholder.typicode.com/posts/1')
    .then(response => response.json())
    .then(response => {
        return fetch('https://jsonplaceholder.typicode.com/posts/2')
            .then(response2 => response2.json())
    });
addTask(myTask); // Here's where we register this task so that prerendering waits for it
return myTask;

This should keep the domain context open for as long as your promise chain is in flight, so it should not clear out the baseUrl context data before it's finished.

@rosieks
Copy link
Contributor Author

rosieks commented May 2, 2017

I was aware of addTask from WeatherForecasts.ts - I have made a few other tests and I noticed that:

  • when I call my function without addTask the error show up in Visual Studio output window but the page is properly displayed in browser.
  • when I call my function with addTask then I see that error in browser as a error page.
    image

@rosieks
Copy link
Contributor Author

rosieks commented May 4, 2017

@SteveSandersonMS - I have published example that shows this issue. To reproduce that:

@SteveSandersonMS
Copy link
Member

@rosieks Thanks - I'll check it soon. Reopening.

@DanHarman
Copy link
Contributor

@SteveSandersonMS Would you mind making addTask return the task fluently as then we can condense code like the above (of which I have a fair amount) to return addTask(myTask);

Great use case for using async await too, although not sure on the projects policy on that.

@rosieks
Copy link
Contributor Author

rosieks commented Jun 12, 2017

I just made another test - I replaced first fetch with:
const delay = timeout => new Promise((resolve, reject) => setTimeout(resolve, timeout));
and it also failed.
It looks like baseUrl() doesn't work with promises in general.

Maybe replacing it to async/await will help?

@exera
Copy link

exera commented Aug 1, 2017

any workaround for this? it looks like when nested fetch is called there is no active domain and baseUrl returns undefined.

@aspnet-hello aspnet-hello transferred this issue from aspnet/JavaScriptServices Dec 17, 2018
@aspnet-hello aspnet-hello added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-spa labels Dec 17, 2018
@SteveSandersonMS
Copy link
Member

Sorry this never got a timely response.

I think the behavior reported here comes down to a Node issue whereby it fails to preserve domain context in native Promise continuations. That's not something we can directly fix, so if you're chaining promises and using addTask, I think you'll unfortunately have to re-assign the baseUrl at the start of each continuation (e.g., by calling baseUrl('http://example.com')).

@mkArtakMSFT Propose we close this as it's an external (Node) bug that we're not planning to work around.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-spa
Projects
None yet
Development

No branches or pull requests

6 participants