pusher / chatkit-client-js

JavaScript client SDK for Pusher Chatkit
https://pusher.com/chatkit
MIT License
90 stars 15 forks source link

addMessage is calling the error handler with a HTTP status code of 201 #3

Closed rrcobb closed 6 years ago

rrcobb commented 6 years ago

A successful call to currentUser.addMessage ends up rejecting instead of resolving on success.

It looks like the request is successful, but the response parsing logic is wrong in executeNetworkRequest in the core. A 201 response is a successful object creation, but the handler is looking for status codes of 200 only, and rejecting otherwise.

In the chrome backtrace,

xhr.onreadystatechange = function () {
            if (xhr.readyState === 4) {
                if (xhr.status === 200) {
                    resolve(xhr.response);
                }
                else if (xhr.status !== 0) {
                    reject(network_1.ErrorResponse.fromXHR(xhr));
                }
                else {
                    reject(new network_1.NetworkError("No Connection"));
                }
            }
        };

(I don't have pusher's sourcemaps on, so not quite sure where this is.)

Not the worst, but surprising that a successful create would end up with the promise rejecting instead of resolving.

troygoode commented 6 years ago

This is also causing currentUser.createRoom to fail.

The line in question can be found here (it needs to be updated to accept a 201 also):

https://github.com/pusher/chatkit-client-js/blob/eb32d4ff4526f35a66e7261d0b11f5debb32ef2d/target/chatkit.js#L979

troygoode commented 6 years ago

It looks like you just need to upgrade they version of pusher-platform as this was fixed there:

https://github.com/pusher/pusher-platform-js/blob/0e54667e4b19e0b01e222de15748a22401a6092a/src/request.ts

hamchapman commented 6 years ago

@rrcobb sorry I didn't see this issue sooner - I had neglected to watch the repo.

@troygoode is correct and we're going to release a new version of pusher-platform-js and the Chatkit JS SDK today!

And thanks for the report!

callum-oakley commented 6 years ago

Should be fixed in 0.1.2 🙂