However, the spec states that 404 is a valid response of this endpoint, in the case where a room is unknown or the server is otherwise unable to peek it. But the error that is ultimately returned isn't this 404, but rather another M_UNRECOGNIZED 404 error from the unstable endpoint.
This just obfuscates the original error, making logs harder to interpret - perhaps we should not just check for 404 on the spec endpoint but also check for M_UNRECOGNIZED before falling back to unstable, and if both the unstable and spec endpoints error, somehow pass on both errors with some information that we re-tried.
Or maybe, since MSC2946 has been spec for quite a while, we just remove the whole fallback to unstable mechanism in the first place?
In
federationclient.go / RoomHierarchy, we fall back to the unstable endpoint if the spec endpoint returns 404.However, the spec states that 404 is a valid response of this endpoint, in the case where a room is unknown or the server is otherwise unable to peek it. But the error that is ultimately returned isn't this 404, but rather another
M_UNRECOGNIZED404 error from the unstable endpoint.This just obfuscates the original error, making logs harder to interpret - perhaps we should not just check for 404 on the spec endpoint but also check for
M_UNRECOGNIZEDbefore falling back to unstable, and if both the unstable and spec endpoints error, somehow pass on both errors with some information that we re-tried.Or maybe, since MSC2946 has been spec for quite a while, we just remove the whole fallback to unstable mechanism in the first place?