search

serviejs / popsicle

Simple HTTP requests for node and the browser
MIT License
246 stars 19 forks source link

ECONNRESET not handled #155

Closed ipetrovic11 closed 2 years ago

ipetrovic11 commented 2 years ago

We are using module https://github.com/mulesoft-labs/js-client-oauth2 which is using popsicle for requests.

ECONNRESET is not well handled with popsicle.

We tested this by using fetch instead of popsicle.

Issue is tested on Node 12+

blakeembrey commented 2 years ago

What is the expected behavior of a connection reset? This library doesn't come with retries built-in, it seems like the request in the OAuth2 library could just be retried.

ipetrovic11 commented 2 years ago

Hi @blakeembrey

I don't think that we are looking for retries here.

We just don't want the application to crash(unhandledException) if ECONNRESET happens. It is not well handled.

Just as showcase we tried using the different request libraries, and for example node-fetch worked just fine.

blakeembrey commented 2 years ago

Can you share the version and stack trace?

blakeembrey commented 2 years ago

It might also help to provide whether it’s HTTP1 or 2, or the domain if you’re unsure and I can check.

blakeembrey commented 2 years ago

By node 12+ does that mean you tested on a version later than 12, or just that you tested on 12 and assume it’s broken on later versions. I believe I have a node 12 related hunch that could cause this related to pipelining when this package was migrated to the native node.js utility instead of an NPM module.

ipetrovic11 commented 2 years ago

@blakeembrey you get read more info here: https://github.com/mulesoft-labs/js-client-oauth2/issues/182#issue-1042063238

We tried with Node 12, 14 ,16

blakeembrey commented 2 years ago

OK, I think I can reproduce it. If you share the installed versions this can be confirmed, but this was fixed in April here: https://github.com/serviejs/popsicle-transport-http/releases/tag/v1.1.4. If you re-install it should have installed the latest version of that package without any issues, unless something has pinned the release specifically to 1.1.3.

blakeembrey commented 2 years ago

Have you tried doing npm rm client-oauth2 && npm install client-oauth2 to get the latest version?

blakeembrey commented 2 years ago

FWIW, here's the original report with the information that resolved it that matches the same bug you've pointed out: https://github.com/serviejs/popsicle-transport-http/issues/8.

ipetrovic11 commented 2 years ago

@blakeembrey this could be it, I can see that in our yarn.lock file there is version 1.1.3

I will keep you posted as soon as it is tested, probably early next week.