Joystream / joystream

Joystream Monorepo
http://www.joystream.org
GNU General Public License v3.0
1.42k stars 115 forks source link

[colossus] fix: unhandled error event on `superagent.get` instance #4998

Closed zeeshanakram3 closed 9 months ago

zeeshanakram3 commented 9 months ago

addresses #4995

mnaamani commented 9 months ago

The fix addresses the node crashing on request errors which is definitely needed.

Some ideas for further enhancements:

  1. Timeout should be a function of object size The original issue showed:

    Sync - fetching data error from https://sieemmastorage.com/storage/api/v1/state/data-objects: Error: Time
    out of 120000ms exceeded

    So the node was configured with a 2minute timeout to fetch. It seems a bit arbitrary and wouldn't work for very large files. In general I think a timeout should be much larger, but a more effective mechanism to drop the connection would be monitoring the download progress. There is a superagent-progress plugin that could be useful.

  2. Keep some state about prior sync run Moreover there isn't any kind of "feedback loop" for the synchronizer worker to make more intelligent selection on next run.

kdembler commented 9 months ago

@mnaamani Was that a timeout for the full request or just a timeout for initial response (without the full content?)

mnaamani commented 9 months ago

@mnaamani Was that a timeout for the full request or just a timeout for initial response (without the full content?)

That two minute timeout looks like it was just for the the fetching of the data-objects list. And is for the full response or "deadline" because code uses this call .timeout(120000)

Now that you ask, there are in-fact two possible timeouts that can be configured the response and deadline with this call for example:

.timeout({
        response: 5000,  // Wait 5 seconds for the server to start sending,
        deadline: 60000, // but allow 1 minute for the file to finish loading.
      })

fyi: @zeeshanakram3 @kdembler