got icon indicating copy to clipboard operation
got copied to clipboard

Using CancelableRequest.cancel with cache causes UncaughtException

Open femshima opened this issue 3 years ago • 2 comments

Describe the bug

  • Got version: 11.8.3
  • Node.js version: 16.13.0
  • OS & version: Windows 10 Home 21H1 19043.1348
import got from "got";

const cache = new Map();
const r = got("https://example.com/", { cache: cache });
r.on('downloadProgress', () => {
    r.cancel();
});

r.catch(e => console.log(e));

In the above code, when I call CancelableRequest.cancel(r.cancel()) with cache in options, ECONNRESET exception is throwed by get-stream package(in addition to CancelError(handled)), which is unhandled even if I put .catch to the returned CancelableRequest.

Error: aborted
    at connResetException (node:internal/errors:691:14)
    at TLSSocket.socketCloseListener (node:_http_client:407:19)
    at TLSSocket.emit (node:events:402:35)
    at TLSSocket.emit (node:domain:475:12)
    at node:net:687:12
    at TCP.done (node:_tls_wrap:580:7)
    at TCP.callbackTrampoline (node:internal/async_hooks:130:17) {
  code: 'ECONNRESET',
  bufferedData: Buffer(0) [Uint8Array] []
}

When I remove cache options, the program only throws CancelError.

According to stacktrace, the exception is throwed in rejectPromise function in get-stream package, called by error callback of pump. https://github.com/sindresorhus/get-stream/blob/main/index.js#L36

const rejectPromise = error => {
	// Don't retrieve an oversized buffer.
	if (error && stream.getBufferedLength() <= BufferConstants.MAX_LENGTH) {
		error.bufferedData = stream.getBufferedValue();
	}
	reject(error);//<-here
};
stream = pump(inputStream, bufferStream(options), error => {
	if (error) {
		rejectPromise(error);
		return;
	}
	resolve();
});

inputStream comes from cacheable-request. https://github.com/lukechilds/cacheable-request/blob/master/src/index.js#L106

Actual behavior

aborted(ECONNRESET) exception is throwed, and it is not handled.

Expected behavior

Cancellation only causes CancelError, not with aborted(ECONNRESET) exception.

Code to reproduce

Runkit: https://runkit.com/femshima/got-cancel-with-cache (For more details, please see https://github.com/femshima/got-cancel-cache)

import got from "got";

const cache = new Map();
const r = got("https://example.com/", { cache: cache });
r.on('downloadProgress', () => {
    r.cancel();
});

r.catch(e => console.log(e));

Checklist

  • [x] I have read the documentation.

  • [x] I have tried my code with the latest version of Node.js and Got.

femshima avatar Nov 20 '21 12:11 femshima

I'm encountering the same thing, but I'm not trying to cancel the request, just trying to read the response body.

langri-sha avatar Feb 25 '22 10:02 langri-sha

I think I have the same issue when the request timeout is triggered

import got from "got"; // version 12.0.3

// 3.5Mb so times out
const url =
  "https://www.interieur.gouv.fr/avotreservice/elections/telechargements/PR2017/resultatsT2/032/002/002com.xml";

got(url, {
  cache: new Map(),
  timeout: {
    request: 500,
  },
})
  .then((res) => {
    if (res.statusCode !== 200) {
      console.error(
        `${res.statusCode} - ${res.statusMessage} - fetching ${url}`
      );
    }

    return res.body.slice(0, 100);
  })
  .catch((err) => {
    console.log("ooooh", err);
  });
node:internal/process/promises:246
          triggerUncaughtException(err, true /* fromPromise */);
          ^

Error: aborted
    at connResetException (node:internal/errors:691:14)
    at TLSSocket.socketCloseListener (node:_http_client:407:19)
    at TLSSocket.emit (node:events:402:35)
    at node:net:687:12
    at TCP.done (node:_tls_wrap:580:7) {
  code: 'ECONNRESET',
  bufferedData: Buffer(2424828) [Uint8Array] [
     60,  63, 120, 109, 108,  32, 118, 101, 114, 115, 105, 111,
    110,  61,  34,  49,  46,  48,  34,  32, 101, 110,  99, 111,
    100, 105, 110, 103,  61,  34,  85,  84,  70,  45,  56,  34,
     63,  62,  10,  60,  69, 108, 101,  99, 116, 105, 111, 110,
     62,  10,  32,  32,  60,  83,  99, 114, 117, 116, 105, 110,
     62,  10,  32,  32,  32,  32,  60,  84, 121, 112, 101,  62,
     80, 114, 195, 169, 115, 105, 100, 101, 110, 116, 105, 101,
    108, 108, 101,  60,  47,  84, 121, 112, 101,  62,  10,  32,
     32,  32,  32,  60,
    ... 2424728 more items
  ]
}

aubergene avatar Apr 22 '22 11:04 aubergene

@aubergene @langri-sha - just FYI that the next version being released in October 22 will have a fix around this. https://github.com/jaredwray/cacheable-request/pull/206

jaredwray avatar Oct 27 '22 16:10 jaredwray

@jaredwray thank you 🙌 !!!

langri-sha avatar Nov 15 '22 18:11 langri-sha