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.

Sign up for GitHub

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

Jump to bottom

Consider running browser tests against FetchRequest in addition to XHRRequest #1772

Open
lawrence-forooghian opened this issue May 21, 2024 · 0 comments
Open

Consider running browser tests against FetchRequest in addition to XHRRequest #1772

lawrence-forooghian opened this issue May 21, 2024 · 0 comments
Labels
testing Includes all kinds of automated tests, the way that we run them and the infrastructure around them.

Comments

@lawrence-forooghian
Copy link
Collaborator

lawrence-forooghian commented May 21, 2024 •
edited by sync-by-unito bot
Loading

We do not currently exercise our FetchRequest class in the tests; they all use XHR, the only exception being some of the tests for the modular variant the SDK. This might be hiding bugs in the FetchRequest implementation such as #1771. See if we could run the tests against FetchRequest as well as XHR, the same way we exercise all of the different transports in (some) tests.

┆Issue is synchronized with this Jira Task by Unito
@lawrence-forooghian lawrence-forooghian added the testing Includes all kinds of automated tests, the way that we run them and the infrastructure around them. label May 21, 2024
@lawrence-forooghian lawrence-forooghian mentioned this issue May 21, 2024
Closed
lawrence-forooghian added a commit that referenced this issue May 21, 2024
@lawrence-forooghian
Make FetchRequest not try and parse a 204 response body
e1c69ef 
It currently checks for the presence of a Content-Type response header
and if present assumes that trying to parse the response will succeed.

Realtime currently sends a 204 (No Content) with a Content-Type header
from the POST rest.ably.io/push/publish endpoint when using HTTP 1.1
(see REA-1924).

(This bug hasn't been caught by the tests because I guess most of the
time they use HTTP 2 in browsers, and also we don't use FetchRequest in
the tests except briefly in those for the modular variant of the
library. I’ve created #1772 for improving this situation).

The 204 handling approach is copied from that which we already have in
XHRRequest.

Resolves #1771.
@lawrence-forooghian lawrence-forooghian mentioned this issue May 21, 2024
Merged
lawrence-forooghian added a commit that referenced this issue May 21, 2024
@lawrence-forooghian
Make FetchRequest not try and parse a 204 response body
80b6506 
It currently checks for the presence of a Content-Type response header
and if present assumes that trying to parse the response will succeed.

Realtime currently sends a 204 (No Content) with a Content-Type header
from the POST rest.ably.io/push/publish endpoint when using HTTP 1.1
(see REA-1924).

(This bug hasn't been caught by the tests because I guess most of the
time they use HTTP 2 in browsers, and also we don't use FetchRequest in
the tests except briefly in those for the modular variant of the
library. I’ve created #1772 for improving this situation).

The 204 handling approach is copied from that which we already have in
XHRRequest.

Resolves #1771.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
testing Includes all kinds of automated tests, the way that we run them and the infrastructure around them.
Milestone
No milestone
Development

No branches or pull requests
1 participant
@lawrence-forooghian