Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Upstream controller don't abort all requests #38

Merged
merged 3 commits into from
Nov 2, 2023
Merged

Upstream controller don't abort all requests #38

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d0ba4f2
Upstream controller don't abort all requests
Nov 2, 2023
e7cfc37
If upstream controller aborted, stop
Nov 2, 2023
9a46ef5
Merge branch 'main' into fix-controller
Nov 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion src/client.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,9 +248,13 @@ export class Saturn {
* @param {string} [opts.url]
* @param {number} [opts.connectTimeout=5000]
* @param {number} [opts.downloadTimeout=0]
* @param {AbortController} [opts.controller]
* @returns {Promise<AsyncIterable<Uint8Array>>}
*/
async * fetchContentWithFallback (cidPath, opts = {}) {
const upstreamController = opts.controller;
delete opts.controller;

let lastError = null
// we use this to checkpoint at which chunk a request failed.
// this is temporary until range requests are supported.
Expand All @@ -261,6 +265,13 @@ export class Saturn {
}

const fetchContent = async function * () {
const controller = new AbortController();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is nonblocking but I would add this in the fetchCid function instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This special handling of the controller is specific to the fallback one (where there are 5 retries), the fetchCid function should use the one passed by the caller. Can you expand on your thoughts?

opts.controller = controller;
if (upstreamController) {
upstreamController.signal.addEventListener('abort', () => {
controller.abort();
});
}
let byteCount = 0
const byteChunks = await this.fetchContent(cidPath, opts)
for await (const chunk of byteChunks) {
Expand Down Expand Up @@ -298,7 +309,7 @@ export class Saturn {
let fallbackCount = 0
const nodes = this.nodes
for (let i = 0; i < nodes.length; i++) {
if (fallbackCount > this.opts.fallbackLimit) {
if (fallbackCount > this.opts.fallbackLimit || upstreamController?.signal.aborted) {
return
}
if (opts.raceNodes) {
Expand Down
Loading