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

Fix bug with the /hierarchy API sending invalid requests #2201

Merged
merged 1 commit into from
Feb 24, 2022
Merged

Fix bug with the /hierarchy API sending invalid requests #2201

merged 1 commit into from
Feb 24, 2022

Conversation

t3chguy
Copy link
Member

@t3chguy t3chguy commented Feb 24, 2022 •
edited by github-actions bot
Loading

Fixes element-hq/element-web#21170

Here's what your changelog entry will look like:

🐛 Bug Fixes

@t3chguy t3chguy requested a review from a team as a code owner February 24, 2022 14:12
@t3chguy t3chguy mentioned this pull request Feb 24, 2022
Merged
andybalaam
andybalaam approved these changes Feb 24, 2022
View reviewed changes
Copy link
Contributor

@andybalaam andybalaam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@t3chguy t3chguy merged commit 735ccca into develop Feb 24, 2022
@t3chguy t3chguy deleted the t3chguy/fix/21170 branch February 24, 2022 14:49
@kerryarchibald
Copy link
Contributor

This should be being taken care of with the http library in matrix-js-sdk, before matrix-org/matrix-react-sdk#7816 the request library was properly stripping undefined query params from the query string. I've been trying to understand how the typing changes could possibly break that behaviour, but reverting that PR does indeed fix the issue.

I suspect we'll find other instances of this problem popping up.

@t3chguy
Copy link
Member Author

t3chguy commented Feb 24, 2022

This should be being taken care of with the http library in matrix-js-sdk

A lot of other callers build their queryParams up this way too

before matrix-org/matrix-react-sdk#7816 the request library was properly stripping undefined query params from the query string. I've been trying to understand how the typing changes could possibly break that behaviour, but reverting that PR does indeed fix the issue.

So its the react-sdk PR, not the js-sdk PR causing this?

@kerryarchibald
Copy link
Contributor

Reverting matrix-org/matrix-react-sdk#7816 with yarn linked matrix-js-sdk at latest develop fixes the issue.

su-ex added a commit to SchildiChat/matrix-js-sdk that referenced this pull request Mar 17, 2022
@su-ex
Merge tag 'v16.0.0' into sc
8daa03b 
* Improve typing around event emitter handlers ([\matrix-org#2180](matrix-org#2180)).
* Fix defer not supporting resolving with a Promise<T> ([\matrix-org#2216](matrix-org#2216)).
* add LocationAssetType enum ([\matrix-org#2214](matrix-org#2214)).
* Support for mid-call devices changes ([\matrix-org#2154](matrix-org#2154)). Contributed by @SimonBrandner.
* Add new room state emit RoomStateEvent.Update for lower-frequency hits ([\matrix-org#2192](matrix-org#2192)).
* Fix wrong event_id being sent for m.in_reply_to of threads ([\matrix-org#2213](matrix-org#2213)).
* Fix wrongly asserting that PushRule::conditions is non-null ([\matrix-org#2217](matrix-org#2217)).
* Make createThread more resilient when missing rootEvent ([\matrix-org#2207](matrix-org#2207)). Fixes element-hq/element-web#21130.
* Fix bug with the /hierarchy API sending invalid requests ([\matrix-org#2201](matrix-org#2201)). Fixes element-hq/element-web#21170.
* fix relation sender filter ([\matrix-org#2196](matrix-org#2196)). Fixes element-hq/element-web#20877.
* Fix bug with one-way audio after a transfer ([\matrix-org#2193](matrix-org#2193)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andybalaam andybalaam andybalaam approved these changes

Assignees
No one assigned
Labels
T-Defect
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Can't browse rooms of a space
3 participants
@t3chguy @kerryarchibald @andybalaam