Cancel the active request instead of the most recent request

[selamanse]

Closes #2690

I replaced the inner network.js variable cancelRequestFunction with a Map that remembers the requestId of the given cancelFunction.

Also the ui now uses cancelRequestById with the actual requestId to cancel the specific request.

I tested package insomnia-app locally via ui with the express based test-server that was proposed by the issue description (very helpful and precise by the way).

I also ran all tests with npm test locally from repository root...all seem fine.

I am unsure about ui or module tests. Is there something I should add?

[develohpanda]

I'm really impressed, brilliant work 👏

I am unsure about ui or module tests. Is there something I should add?

Currently there are no UI tests (this is WIP as I'd like to add them too).

Some tests could potentially be added for the cancelRequestById function in network.test.js though. Looks like there are only tests for _actuallySend, which sets up the cancellation logic, so we might be able to add a few test cases to also cancel.

Give that a go, and if it's non-trivial to add tests for cancellation then I'll merge this in as-is. Looks pretty good to me!

[develohpanda]

Small change about clearing the cancel function once a request completes or is cancelled, to avoid a memory leak 😄

[develohpanda]

Your update to the unit test was exactly what I had in mind, nice work!

A couple of comments, almost there 🤗

[develohpanda]

Nicely done 💯

[develohpanda]

I did some manual testing:

• cancelled requests
• successful requests
• failed requests (invalid url)

Cancel functions didn't persist longer than necessary. So excited to merge this!

[selamanse]

@develohpanda thanks for reviewing and advising. I appreciate this.

[develohpanda]

Credit where credit is due, @DMarby noticed the memory leak, and you did a great job with fixing it up.

I was certainly impressed with your refined unit test as well! 🥇