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

Connection issues #20

Closed
mattheworiordan opened this issue Mar 14, 2015 · 14 comments
Closed

Connection issues #20

mattheworiordan opened this issue Mar 14, 2015 · 14 comments
Assignees
Labels
bug Something isn't working. It's clear that this does need to be fixed.

Comments

@mattheworiordan
Copy link
Member

@paddybyers I am a bit confused why http://jsbin.com/vuvozisape/1/edit?js,console,output is not working.

Seemingly it will only connect when using the format Ably.Realtime({ key: "key" }), and all other key & token formats fail for different reasons.

I also thought we had changed our libraries to support the use of a a key or token string in place of the constructor options which too doesn't seem to be working.

Can we ensure we add test coverage to this once resolved?

@mattheworiordan mattheworiordan added the bug Something isn't working. It's clear that this does need to be fixed. label Mar 14, 2015
@paddybyers
Copy link
Member

This is fixed:
2ad36b1

we support

Realtime({key: <key>})
Realtime({authToken: <token>})
Realtime(<key>)
Realtime(<token>)

I've added tests for the string-only cases for realtime and rest

@mattheworiordan
Copy link
Member Author

No tests to prevent a regression?

@mattheworiordan
Copy link
Member Author

Also, is this deployed so I can retry the JSBin?

@paddybyers
Copy link
Member

No tests to prevent a regression?

I added tests, as stated above

@paddybyers
Copy link
Member

Also, is this deployed so I can retry the JSBin?

Not yet

@mattheworiordan
Copy link
Member Author

Can't see any tests in the commit you sent? Given 3 out of 4 were failing, should we not add coverage to all? Can you deploy please so that we can test this + the other auth issue in production and then in CI for the web server?

@paddybyers
Copy link
Member

Can't see any tests in the commit you sent?

79848f8

Given 3 out of 4 were failing, should we not add coverage to all?

2 were failing (tests above), 1 was being used incorrectly (token: xx instead of authToken: xx).

@mattheworiordan
Copy link
Member Author

Ah, great, shout when this and other fixes are deployed so that we can test.

@mattheworiordan
Copy link
Member Author

Unfortunately there is another bug, if you pass in an API key without the form key: 'key' then the client tries to connect over HTTP. See https://jsbin.ably.io/ogizus/3/edit which works, and https://jsbin.ably.io/ogizus/2/edit which fails.

@paddybyers
Copy link
Member

This should fix it: c458536

@mattheworiordan
Copy link
Member Author

I see that's tagged 0.7.5, is that deployed?
Hate to be a stickler, but no test to prevent a regression?

@paddybyers
Copy link
Member

I see that's tagged 0.7.5, is that deployed?

Yes

Hate to be a stickler, but no test to prevent a regression?

e1f0e89

@paddybyers
Copy link
Member

Resolved

@mattheworiordan
Copy link
Member Author

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working. It's clear that this does need to be fixed.
Development

No branches or pull requests

2 participants