feat: Track and allow querying in-flight requests #186

tehhowch · 2021-12-04T18:28:52Z

Start tracking a request as soon as it is issued, rather than only after it has completed
Lay groundwork for future waitForPendingRequests option from Feature Request: allow querying for in-progress HTTP calls #111
- consumers can write their own version now by wrapping the new browser.hasPendingRequests() command
deprecates the boolean param to assertExpectedRequestsOnly
more tests!

I took care to preserve the current behavior of the library w.r.t getRequest / getRequests ordering, which (despite the previous README's statement) always reported only completed requests, and always ordered by the time of completion. With the new functionality, the next major version of this library could change the default ordering to time of initiation, and could default to waiting for pending requests to settle, etc, -- which are probably the expected behavior for someone new to this library.

Closes #111
Ref #149 #140

- update middleware to support delayed responses

- `addRequest` always sets `pending: false` - while `typeof null === 'object'`, clarify that body and response.body can be null.

- if not passing options object (all past code), will continue to receive completed requests only - if passing options object with includePending = false, will receive completed requests only - if passing options object with includePending = true, will receive all requests.

- refactor tests for pending requests into own describe block, and add explicit waits to avoid affecting following tests.

…tion Support function overloads in future work

…ract method for getting contents - add runtime check for incomplete polyfill scenario, where fetch is polyfilled but Promise.all is not.

…uests & parse the response body only if marked as fulfilled

- in sync mode, these were awaited implicitly. In async mode, however, they must be explicitly awaited. - Increase assertion strictness on the matched errors - due to an issue with how assert.rejects forwards a regexp containing an escaped regexp, use a validation function to directly invoke assert.matches with the regexp

- add requests prior to invoking window._fetch - complete requests after the full response body is available

- note: must stringify `lastURL` in case XHR#open is invoked with a URL object.

- replace `if-else if` chain with `if + return` - log the serialization error, if any

consumers should be able to easily identify the source of an issue when they are running tests Ideally these logs would be passed to the node / test context, rather than emitted in the tested browser, but depending on the test runner they may be surfaced without much additional work

This is the existing behavior as of 4.1.10 and all earlier versions

simplify assignment to `request` in node context

The behavior in 4.1.10 and earlier versions is that requests are ordered by time of fulfillment (`orderBy: 'END'`). This should remain the default behavior, but users should be able to order by the time the request started as well (which is possibly more under their control, in terms of testing, too). When pending requests are requested, this default sort will place them last (as they are not yet "completed").

…ny request is pending

… methods - deprecate boolean param to assertExpectedRequestsOnly - pass `inOrder` as an option instead - don't suggest asserting on pending requests will work - Since a pending request has no response, it has no response status code. (Passing `includePending: true` would result in a TypeError in the library code). - A better approach will be to support an option that requires pending requests are completed before performing assertions.

…iver

tehhowch · 2021-12-04T18:29:13Z

@chmanie @christian-bromann please take a look!

tehhowch · 2021-12-05T16:06:22Z

types/wdio-intercept-service.d.ts

 }
 /**
 * Convert T to T or Promise<T> depending if `@wdio/sync` is being used or not.
 */
-type AsyncSync<T> = WebdriverIO.BrowserObject extends WebDriver.Client
+type AsyncSync<T> = WebdriverIO.Browser extends WebDriver.Client


@christian-bromann I don't believe this configuration is correct -- I had turned on library type checking and it complained that the WebdriverIO namespace didn't have an entry for BrowserObject, but was fine with Browser instead. There is still an error for Client not being found in the Webdriver namespace.

With & without this change to the AsyncSync conditional type, it seems this library's types always evaluate to "sync mode" in a consuming TypeScript project:

How might this library (and others that want to provide accurate types for custom commands) best support sync & async modes in WebdriverIO versions 5, 6, and 7 (or at minimum v6 and later, as v6 is still officially a supported version)?

(Existing issues suggest that this is not a new issue with this library, though, e.g. #148 #64 #80 )

How might this library (and others that want to provide accurate types for custom commands) best support sync & async modes in WebdriverIO versions 5, 6, and 7 (or at minimum v6 and later, as v6 is still officially a supported version)?

With the help of the community. While it would be amazing to provide best support for sync (with or without typescript) I doubt that we have enough resources to do so. Therefor we are happy if someone helps us with pull requests.

I asked since you are most familiar with the wdio types found in both V6 and V7 and should be most able to tell the community what WDIO interface hierarchy is found in both sync v6 and sync v7 and thus can be used for sync detection in a d.ts file (since we cannot do a require('@wdio/sync') test).

christian-bromann

The change looks generally good to me. Thanks for all the work 👍 👏

Is there a way we can some e2e tests for it?
Given I am not too familiar with the code base it would be great if @chmanie can chime in for an additional review.

tehhowch · 2021-12-06T12:28:05Z

Is there a way we can some e2e tests for it?

@christian-bromann This PR does add quite a few E2E tests to that file, would you mind clarifying?

christian-bromann · 2021-12-06T12:34:07Z

@tehhowch you are right, I was overseeing them because GitHub collapsed them. All good to me them. Let's give @chmanie some time to chime in and if there is no feedback I will go ahead and merge. Thanks so much!

chmanie

This is amazing work! Thank you so much for putting in the effort, this will be super helpful to a lot of people. Generally this looks good to me, even though I added some comments (we should especially look into the case for a major version bump). Could you also amend the changelog, like so (we're not doing this yet, but it'd be super helpful):

### [ [>](https://github.com/chmanie/wdio-intercept-service/tree/v.next) ] v.next / <DATE>
- Describe changes here (thanks to @tehhowch)

chmanie · 2021-12-06T13:07:14Z

README.md

-* `request.response.headers`: response http headers as JS object
-* `request.response.body`: response body (will be parsed as JSON if possible)
-* `request.response.statusCode`: response status code
+* `request.pending`: boolean flag for whether this request is complete (i.e. has a `response` property), or in-flight.


README.md

chmanie · 2021-12-06T13:10:30Z

index.js

-      return getRequest().then((requests) => {
+
+      // Don't let users request pending requests:
+      if (options.includePending) {


Very nice catch here, providing a less frustrating experience :)

chmanie · 2021-12-06T13:12:15Z

index.js

      const expectations = this._wdajaxExpectations;
+      let inOrder = true;
+      let options = {};
+      if (typeof orderOrOptions === 'boolean') {


Honestly, I think I would be OK with changing the API (like in the readme) and bumping the major version

I vastly prefer the approach of Ember.JS, where the framework's policy is that users should be able to upgrade to the next major versions without their app breaking so as long as their app runs the previous version without encountering deprecation warnings. That is, a semver major bump should never add functionality, it should only remove already-deprecated functionality.

Yeah okay, I see your point. The downside here is that the actual API is not necessarily 100% consistent with the actual code but that might only cause confusion for people who actually work on the code and not for the ones using it. All in all I'd be fine with this.

chmanie · 2021-12-06T13:13:50Z

index.js

+        body: parseBody(req.requestBody),
+        pending: true,
+      };
+      // Check for a '__fulfilled' property on the retrieved request, which is


chmanie · 2021-12-06T13:15:30Z

lib/interceptor.js

      window.sessionStorage.removeItem(NAMESPACE);
    }

    if (typeof window.fetch == 'function') {
      replaceFetch();
+      if (
+        typeof window.Promise === 'undefined' ||


Is there ever a case where window.fetch exists, but window.Promise does not? I can only imagine very weird (and broken!) cases of polyfilling.

There shouldn't, but I could imagine that someone might polyfill fetch, Promise.then, and Promise.catch, but not polyfill Promise.all. I figured it was better that we detect this scenario and throw promptly, rather than fail when the fetch wrapper is invoked because of this library's use of Promise.all

lib/interceptor.js

…ral methods

tehhowch · 2021-12-06T15:20:41Z

@chmanie updated the changelog (I added a couple of entries for the latest releases too). It may be worth changing to use an automated changelog generator, since that seems to be the common approach for webdriverio projects as well

chmanie · 2021-12-06T15:34:20Z

Good work! All good from my side! Feel free to merge away

tehhowch · 2021-12-07T15:08:37Z

@christian-bromann ^

christian-bromann

Awesome! Thanks for the review @chmanie and the work @tehhowch 👍

tehhowch added 20 commits December 4, 2021 11:50

test(pending): add tests for pending requests

e53e296

- update middleware to support delayed responses

chore(types): update shape of InterceptedRequest

4d5a49b

- `addRequest` always sets `pending: false` - while `typeof null === 'object'`, clarify that body and response.body can be null.

feat: add hasPendingRequests test method

e5c8158

- refactor tests for pending requests into own describe block, and add explicit waits to avoid affecting following tests.

refactor(types): switch from property declaration to function declara…

06bec12

…tion Support function overloads in future work

refactor(sessionStorage): add helper method for support checking, ext…

d543255

…ract method for getting contents - add runtime check for incomplete polyfill scenario, where fetch is polyfilled but Promise.all is not.

feat(getRequest): attach pending property to incomplete retrieved req…

2ac7125

…uests & parse the response body only if marked as fulfilled

feat(fetch-api): add basic support for pending request queries

0eb905a

- add requests prior to invoking window._fetch - complete requests after the full response body is available

feat(xhr-api): add basic support for pending request queries

37ff5c8

- note: must stringify `lastURL` in case XHR#open is invoked with a URL object.

refactor(xhr-api): simplify parsePayload method

d39a8cb

- replace `if-else if` chain with `if + return` - log the serialization error, if any

test(getRequest): add test for includePending: false (default case)

e8971c5

This is the existing behavior as of 4.1.10 and all earlier versions

feat(getRequest): wire up the includePending request option

87d3d01

simplify assignment to `request` in node context

feat(request-order): wire up orderBy parameter in interceptor lib

98b87b5

refactor: Avoid fetching full request list from browser to check if a…

7cefc9d

…ny request is pending

test(firefox): bump slow delay to 1000 ms to account for slower FF dr…

d23bfab

…iver

docs: update README with latest API changes

caabd77

docs: update comment, formatting

c3bfb5e

tehhowch commented Dec 5, 2021

View reviewed changes

christian-bromann reviewed Dec 6, 2021

View reviewed changes

chmanie reviewed Dec 6, 2021

View reviewed changes

tehhowch added 3 commits December 6, 2021 08:11

refactor: add package prefix constant for logs, errors

ded6f2b

docs: Update README to better describe configuration options for seve…

36ca0dc

…ral methods

docs: update changelog with release summaries

03874f5

christian-bromann approved these changes Dec 7, 2021

View reviewed changes

christian-bromann merged commit e0e9179 into webdriverio-community:main Dec 7, 2021

tehhowch deleted the features/track-pending-requests branch December 7, 2021 18:28

tehhowch mentioned this pull request Dec 8, 2021

Question : How can we wait using this service until all/particular network request is completed ? #149

Open

DrewciferB mentioned this pull request Jan 31, 2022

Any example using it wdio7? #148

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Track and allow querying in-flight requests #186

feat: Track and allow querying in-flight requests #186

tehhowch commented Dec 4, 2021

tehhowch commented Dec 4, 2021

tehhowch Dec 5, 2021 •

edited

Loading

christian-bromann Dec 6, 2021

tehhowch Dec 6, 2021

christian-bromann left a comment •

edited

Loading

tehhowch commented Dec 6, 2021

christian-bromann commented Dec 6, 2021

chmanie left a comment

chmanie Dec 6, 2021

chmanie Dec 6, 2021

chmanie Dec 6, 2021

tehhowch Dec 6, 2021

chmanie Dec 6, 2021

chmanie Dec 6, 2021

chmanie Dec 6, 2021

tehhowch Dec 6, 2021

tehhowch commented Dec 6, 2021

chmanie commented Dec 6, 2021

tehhowch commented Dec 7, 2021

christian-bromann left a comment

feat: Track and allow querying in-flight requests #186

feat: Track and allow querying in-flight requests #186

Conversation

tehhowch commented Dec 4, 2021

tehhowch commented Dec 4, 2021

tehhowch Dec 5, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

christian-bromann left a comment • edited Loading

Choose a reason for hiding this comment

tehhowch commented Dec 6, 2021

christian-bromann commented Dec 6, 2021

chmanie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tehhowch commented Dec 6, 2021

chmanie commented Dec 6, 2021

tehhowch commented Dec 7, 2021

christian-bromann left a comment

Choose a reason for hiding this comment

tehhowch Dec 5, 2021 •

edited

Loading

christian-bromann left a comment •

edited

Loading