Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

send 404 as http-status when filter-id is unknown to the server #2380

Merged
merged 12 commits into from
Oct 10, 2019

Conversation

krombel
Copy link
Contributor

@krombel krombel commented Jul 20, 2017

This fixed the weirdness of 400 vs 404 as http status code in the case
the filter id is not known by the server.
E.g. matrix-js-sdk expects 404 here to catch this situation.

See the discussion in #matrix-dev:matrix.org

This fixed the weirdness of 400 vs 404 as http status code in the case
the filter id is not known by the server.
As e.g. matrix-js-sdk expects 404 to catch this situation this leads
to unwanted behaviour.
@matrixbot
Copy link
Member

Can one of the admins verify this patch?

2 similar comments
@matrixbot
Copy link
Member

Can one of the admins verify this patch?

@matrixbot
Copy link
Member

Can one of the admins verify this patch?

krombel and others added 3 commits July 20, 2017 18:02
respond 404 - No such filter with errorcode "M_NOT_FOUND" instead of
404 - No such row with errorcode "M_UNKNOWN" when filter-id is unknown
to the server
@richvdh
Copy link
Member

richvdh commented May 8, 2018

@matrixbot test this please

@richvdh richvdh requested a review from a team July 26, 2019 12:13
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

the changes to the GET api look ok, but I'm unconvinced by the change to /sync

synapse/rest/client/v2_alpha/sync.py Outdated Show resolved Hide resolved
synapse/rest/client/v2_alpha/filter.py Outdated Show resolved Hide resolved
synapse/rest/client/v2_alpha/filter.py Outdated Show resolved Hide resolved
@richvdh
Copy link
Member

richvdh commented Jul 29, 2019

I can't believe it's taken us over 2 years to land this patch. @anoadragon453 are you able to take it on?

changelog.d/2380.bugfix Outdated Show resolved Hide resolved
@richvdh
Copy link
Member

richvdh commented Oct 3, 2019

I'm fed up with the fact that this trivial PR has been bitrotting for so long, and am going to fix it up and land it.

@codecov
Copy link

codecov bot commented Oct 3, 2019

Codecov Report

❗ No coverage uploaded for pull request base (develop@da815c1). Click here to learn what that means.
The diff coverage is 25%.

@@            Coverage Diff             @@
##             develop    #2380   +/-   ##
==========================================
  Coverage           ?   63.31%           
==========================================
  Files              ?      331           
  Lines              ?    36433           
  Branches           ?     6018           
==========================================
  Hits               ?    23069           
  Misses             ?    11722           
  Partials           ?     1642

@richvdh richvdh self-assigned this Oct 4, 2019
@richvdh richvdh merged commit 2efd050 into matrix-org:develop Oct 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants