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

Finish 405 responses to avoid hanging #429

Closed
wants to merge 2 commits into from
Closed

Conversation

turt2live
Copy link
Member

Pull Request Checklist

@turt2live turt2live requested a review from kegsay May 3, 2024 17:18
@turt2live
Copy link
Member Author

test failure appears to be not my fault

@turt2live
Copy link
Member Author

this patch doesn't appear to work: it still hangs for me. I don't know why.

@turt2live turt2live removed the request for review from kegsay May 3, 2024 17:46
@turt2live turt2live marked this pull request as draft May 3, 2024 17:46
turt2live added a commit to matrix-org/matrix-react-sdk that referenced this pull request May 3, 2024
The `OPTIONS` approach from #12492 doesn't work because Synapse *always* responds with 204 (success) to `OPTIONS` requests, as described here: element-hq/synapse#17153

We further can't use `HEAD` because it's not part of the allowed CORS methods, meaning the browser will mask the exact status code and error message from us, and the proxy hangs on the request anyways: matrix-org/sliding-sync#429

To avoid these problems, we instead search for an unstable feature flag to be exposed by the server. Presence of this flag denotes native support. See https://github.com/matrix-org/matrix-spec-proposals/pull/3575/files#r1588877046 for details.

Implementations which support sliding sync natively will need to update to support this new unstable feature flag usage.
github-merge-queue bot pushed a commit to matrix-org/matrix-react-sdk that referenced this pull request May 3, 2024
…12498)

* Check native sliding sync support against an unstable feature flag

The `OPTIONS` approach from #12492 doesn't work because Synapse *always* responds with 204 (success) to `OPTIONS` requests, as described here: element-hq/synapse#17153

We further can't use `HEAD` because it's not part of the allowed CORS methods, meaning the browser will mask the exact status code and error message from us, and the proxy hangs on the request anyways: matrix-org/sliding-sync#429

To avoid these problems, we instead search for an unstable feature flag to be exposed by the server. Presence of this flag denotes native support. See https://github.com/matrix-org/matrix-spec-proposals/pull/3575/files#r1588877046 for details.

Implementations which support sliding sync natively will need to update to support this new unstable feature flag usage.

* Appease the linter

* Appease the tests
@kegsay
Copy link
Member

kegsay commented May 7, 2024

I was going to say that this should make no difference. ServeHTTP simply asks that you call WriteHeader, it doesn't mandate you set headers, read the req body or set a response body.

@kegsay
Copy link
Member

kegsay commented May 13, 2024

Nothing to do here.

@kegsay kegsay closed this May 13, 2024
@turt2live turt2live deleted the travis/close-405 branch May 13, 2024 14:31
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