From e49b68d68d0d1c6fada5f92dc8ccb5c6d87d48f6 Mon Sep 17 00:00:00 2001 From: Khafra Date: Tue, 29 Nov 2022 02:27:56 -0500 Subject: [PATCH] fix(fetch): connection is cancelled when redirecting (#1787) --- lib/fetch/index.js | 5 ++++- lib/fetch/response.js | 2 +- test/fetch/redirect.js | 29 +++++++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 test/fetch/redirect.js diff --git a/lib/fetch/index.js b/lib/fetch/index.js index 8014d0fb3ba..56fe31a13c8 100644 --- a/lib/fetch/index.js +++ b/lib/fetch/index.js @@ -781,8 +781,11 @@ async function mainFetch (fetchParams, recursive = false) { // https://fetch.spec.whatwg.org/#concept-scheme-fetch // given a fetch params fetchParams async function schemeFetch (fetchParams) { + // Note: since the connection is destroyed on redirect, which sets fetchParams to a + // cancelled state, we do not want this condition to trigger *unless* there have been + // no redirects. See https://github.com/nodejs/undici/issues/1776 // 1. If fetchParams is canceled, then return the appropriate network error for fetchParams. - if (isCancelled(fetchParams)) { + if (isCancelled(fetchParams) && fetchParams.request.redirectCount === 0) { return makeAppropriateNetworkError(fetchParams) } diff --git a/lib/fetch/response.js b/lib/fetch/response.js index fc6746bfa8d..09732114e7a 100644 --- a/lib/fetch/response.js +++ b/lib/fetch/response.js @@ -436,7 +436,7 @@ function makeAppropriateNetworkError (fetchParams) { // otherwise return a network error. return isAborted(fetchParams) ? makeNetworkError(new DOMException('The operation was aborted.', 'AbortError')) - : makeNetworkError(fetchParams.controller.terminated.reason) + : makeNetworkError('Request was cancelled.') } // https://whatpr.org/fetch/1392.html#initialize-a-response diff --git a/test/fetch/redirect.js b/test/fetch/redirect.js new file mode 100644 index 00000000000..c43ee482133 --- /dev/null +++ b/test/fetch/redirect.js @@ -0,0 +1,29 @@ +'use strict' + +const { test } = require('tap') +const { createServer } = require('http') +const { once } = require('events') +const { fetch } = require('../..') + +// https://github.com/nodejs/undici/issues/1776 +test('Redirecting with a body does not cancel the current request - #1776', async (t) => { + const server = createServer((req, res) => { + if (req.url === '/redirect') { + res.statusCode = 301 + res.setHeader('location', '/redirect/') + res.write('Moved Permanently') + setTimeout(() => res.end(), 500) + return + } + + res.write(req.url) + res.end() + }).listen(0) + + t.teardown(server.close.bind(server)) + await once(server, 'listening') + + const resp = await fetch(`http://localhost:${server.address().port}/redirect`) + t.equal(await resp.text(), '/redirect/') + t.ok(resp.redirected) +})