Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core-http] timeout isn't working on the browser #5747

Closed
1 of 6 tasks
sadasant opened this issue Oct 23, 2019 · 7 comments
Closed
1 of 6 tasks

[core-http] timeout isn't working on the browser #5747

sadasant opened this issue Oct 23, 2019 · 7 comments
Assignees
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library.

Comments

@sadasant
Copy link
Contributor

  • Package Name: core-http
  • Package Version: 1.0.0-preview.6
  • Operating system: Linux, Debian Stretch
  • nodejs
    • version: v10.16.0 (it works on Node though)
  • browser
    • name/version: Chrome headless, the one that can be used by Karma. We're using Karma 4.0.1 and karma-chrome-launcher 3.0.0
  • typescript
    • version: Latest (this is fine)
  • Is the bug related to documentation in

Describe the bug
The timeout option of RequestOptionsBase, if sent to an http request over the browser, is not properly working, as in: the timeout never happens, the request goes through as if this option wasn't present.

To Reproduce
Steps to reproduce the behavior:

  1. Use the keyvault-keys package, edit the tests that use timeout to work for the browsers too, then run the tests.

Expected behavior
The requests should trigger the timeout.

@HarshaNalluru
Copy link
Member

Why didn't we such issues with storage-packages, we don't have tests in storage that covers this?

@HarshaNalluru
Copy link
Member

@sadasant, can you provide me with a branch that repros this?

@HarshaNalluru
Copy link
Member

HarshaNalluru commented Jan 13, 2020

For reference - https://github.com/Azure/azure-sdk-for-js/blob/master/sdk/keyvault/keyvault-certificates/test/CRUD.test.ts#L79-L97 from @sadasant

So, the test is being currently run only in node and live, rest of the combinations fail.

  • node-live (yes ✔)
  • node-playback (no ❌)
  • browser-live (no ❌)
  • browser-playback (no ❌)

@sadasant
Copy link
Contributor Author

sadasant commented Jan 24, 2020

Here's something interesting.

If we set the following keyvault-keys to .only (and remove any conditionals around them), they do pass on record and playback for Node:

it.only("can create a key with requestOptions timeout", async function() {
}
it.only("can create a RSA key with requestOptions timeout", async function() {
}
it.only("can create an EC key with requestOptions timeout", async function() {
}

However, if after doing that one does .only to a previously passing test (I mean one that would already pass on record and playback on master), that test passes, but the others with .only don't. Example: Let's say we add .only on the first test of that file:

it.only("can create a key while giving a manual type", async function() {
}

Then that one will pass, and the others with timeout will crash.

@sadasant sadasant reopened this Jan 24, 2020
@sadasant
Copy link
Contributor Author

I closed this by mistake.

@HarshaNalluru
Copy link
Member

Notes

The issue is requestOptions timeout test is failing in the browser while it succeeds in node.
image

The test expects AbortError to be thrown.
Upon some digging through the internals of it in core-http, found this..
In node, we do a setTimeout with abort().
image

In browser, we are throwing a new error instead of the AbortError - This is the only difference.
image

This is the reason for the test failure.

@HarshaNalluru
Copy link
Member

Closing with #7159
Scoping this issue to core-http browser issue.

Logging separate issues for node and browser playback.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library.
Projects
None yet
Development

No branches or pull requests

4 participants