helperdiscord / petitio

An HTTP/S library designed to be simple, fast, and type-strong.
https://helperdiscord.github.io/petitio/
MIT License
44 stars 6 forks source link

docs: improve send throw docs #29

Closed diced closed 3 years ago

diced commented 3 years ago

It was a bit of a pain to figure out what would be thrown if a timeout was reached, yet figured out it was HeadersTimeoutError, BodyTimeoutError or a ConnectTimeoutError. I added a description to the timeout method that explains that it throws that error.

tbnritzdoge commented 3 years ago

@Nytelife26 correct me if I'm wrong but this PR is generally pointless because there are many errors that could occur and these timeout errors would still happen even if you didn't use PetitioRequest#timeout since there are defaults, and as requesting is promise based users should be catching errors anyways.

Nytelife26 commented 3 years ago

these timeout errors would still happen even if you didn't use PetitioRequest#timeout since there are defaults

It is quite well in-scope for this PR to document those too, then.

and as requesting is promise based users should be catching errors anyways

They should be handling errors, yes. It is hard to create specific handlers without knowing the error types in advance, though.

Nytelife26 commented 3 years ago

technically when it does throw a timeout it's not from timeout right?

Correct. The timeout throwing documentation should probably go under PetitioRequest#send, and be linked with @.***on PetitioRequest#timeout` if that's okay.

Thanks in advance.

diced commented 3 years ago

technically when it does throw a timeout it's not from timeout right?

Correct. The timeout throwing documentation should probably go under PetitioRequest#send, and be linked with @.***on PetitioRequest#timeout` if that's okay.

Thanks in advance.

Alright just wanted to clear that up, I'll add these when I have time today

Nytelife26 commented 3 years ago

@diced Status on this? It's been a while.

diced commented 3 years ago

@diced Status on this? It's been a while.

Sorry forgot about it since, I wasn't able to commit due to the tests not succeeding because one of the websites used in the tests was down at the time, I'll make a commit by tonight. (Seems to work now !)

Nytelife26 commented 3 years ago

Sorry forgot about it since, I wasn't able to commit due to the tests not succeeding because one of the websites used in the tests was down at the time, I'll make a commit by tonight.

Ah. Sincerest apologies then, good to hear from you.

As for that, I'm working on internalizing testing by using a local development server, so that shouldn't be an issue in the future.

github-actions[bot] commented 3 years ago

:tada: This PR is included in version 1.3.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket: