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

Support request in the browser and support supplying servers to try in joinRoom() #764

Merged
merged 8 commits into from
Oct 26, 2018

Conversation

turt2live
Copy link
Member

@turt2live turt2live commented Oct 25, 2018

This has a bug when using browser-request where the query string for `server_name: [a, b]` comes out as `?server_name=a,b` instead of `?server_name=a&server_name=b`. This is due to browser-request not supporting the same qs options as request, so the qsStringifyOptions do nothing.
@ara4n
Copy link
Member

ara4n commented Oct 26, 2018

lgtm, other than paranoia about what weird regressions switching HTTP client is going to introduce. Particularly worried about whether timeout behaviour is going to change, or unexpected headers being introduced/removed by the lib etc. Timeouts particularly won't be caught by E2E tests or UTs. Might be worth checking for paranoia's sake?

@turt2live
Copy link
Member Author

I walked around the app and didn't encounter any obvious issues. I also knocked out synapse to get a connection refused error and that worked. Also knocked out the entire VPS synapse runs on and the timeouts appear to work.

@turt2live turt2live merged commit 01e31af into develop Oct 26, 2018
@turt2live turt2live deleted the travis/permalink-routing branch October 26, 2018 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants