matrix/matrix-js-sdk
1
0
Fork 0
mirror of https://github.com/matrix-org/matrix-js-sdk.git synced 2025-08-06 12:02:40 +03:00
Code Activity

Capture HTTP error response headers & handle Retry-After header (MSC4041) (#4471)

Browse Source 
* Include HTTP response headers in MatrixError

* Lint

* Support MSC4041 / Retry-After header

* Fix tests

* Remove redundant MatrixError parameter properties

They are inherited from HTTPError, so there is no need to mark them as
parameter properties.

* Comment that retry_after_ms is deprecated

* Properly handle colons in XHR header values

Also remove the negation in the if-condition for better readability

* Improve Retry-After parsing and docstring

* Revert ternary operator to if statements

for readability

* Reuse resolved Headers for Content-Type parsing

* Treat empty Content-Type differently from null

* Add MatrixError#isRateLimitError

This is separate from MatrixError#getRetryAfterMs because it's possible
for a rate-limit error to have no Retry-After time, and having separate
methods to check each makes that more clear.

* Ignore HTTP status code when getting Retry-After

because status codes other than 429 may have Retry-After

* Catch Retry-After parsing errors

* Add test coverage for HTTP error headers

* Update license years

* Move safe Retry-After lookup to global function

so it can more conveniently check if an error is a MatrixError

* Lint

* Inline Retry-After header value parsing

as it is only used in one place and doesn't need to be exported

* Update docstrings

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

* Use bare catch

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

* Give HTTPError methods for rate-limit checks

and make MatrixError inherit them

* Cover undefined errcode in rate-limit check

* Update safeGetRetryAfterMs docstring

Be explicit that errors that don't look like rate-limiting errors will
not pull a retry delay value from the error.

* Use rate-limit helper functions in more places

* Group the header tests

---------

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
This commit is contained in:
Andrew Ferrazzutti
2024-10-30 11:52:34 -04:00
committed by GitHub
parent 16153e5d82
commit 546047a050
8 changed files with 345 additions and 82 deletions
Download Patch File Download Diff File Expand all files Collapse all files

142
spec/unit/http-api/utils.spec.ts
View File

@@ -1,5 +1,5 @@
/*
Copyright 2022 The Matrix.org Foundation C.I.C.
Copyright 2022 - 2024 The Matrix.org Foundation C.I.C.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
@@ -86,13 +86,28 @@ describe("anySignal", () => {
});
describe("parseErrorResponse", () => {
let headers: Headers;
const xhrHeaderMethods = {
getResponseHeader: (name: string) => headers.get(name),
getAllResponseHeaders: () => {
let allHeaders = "";
headers.forEach((value, key) => {
allHeaders += `${key.toLowerCase()}: ${value}\r\n`;
});
return allHeaders;
},
};
beforeEach(() => {
headers = new Headers();
});
it("should resolve Matrix Errors from XHR", () => {
headers.set("Content-Type", "application/json");
expect(
parseErrorResponse(
{
getResponseHeader(name: string): string | null {
return name === "Content-Type" ? "application/json" : null;
},
...xhrHeaderMethods,
status: 500,
} as XMLHttpRequest,
'{"errcode": "TEST"}',
@@ -108,14 +123,11 @@ describe("parseErrorResponse", () => {
});
it("should resolve Matrix Errors from fetch", () => {
headers.set("Content-Type", "application/json");
expect(
parseErrorResponse(
{
headers: {
get(name: string): string | null {
return name === "Content-Type" ? "application/json" : null;
},
},
headers,
status: 500,
} as Response,
'{"errcode": "TEST"}',
@@ -131,13 +143,12 @@ describe("parseErrorResponse", () => {
});
it("should resolve Matrix Errors from XHR with urls", () => {
headers.set("Content-Type", "application/json");
expect(
parseErrorResponse(
{
responseURL: "https://example.com",
getResponseHeader(name: string): string | null {
return name === "Content-Type" ? "application/json" : null;
},
...xhrHeaderMethods,
status: 500,
} as XMLHttpRequest,
'{"errcode": "TEST"}',
@@ -154,15 +165,12 @@ describe("parseErrorResponse", () => {
});
it("should resolve Matrix Errors from fetch with urls", () => {
headers.set("Content-Type", "application/json");
expect(
parseErrorResponse(
{
url: "https://example.com",
headers: {
get(name: string): string | null {
return name === "Content-Type" ? "application/json" : null;
},
},
headers,
status: 500,
} as Response,
'{"errcode": "TEST"}',
@@ -178,6 +186,66 @@ describe("parseErrorResponse", () => {
);
});
describe("with HTTP headers", () => {
function addHeaders(headers: Headers) {
headers.set("Age", "0");
headers.set("Date", "Thu, 01 Jan 1970 00:00:00 GMT"); // value contains colons
headers.set("x-empty", "");
headers.set("x-multi", "1");
headers.append("x-multi", "2");
}
function compareHeaders(expectedHeaders: Headers, otherHeaders: Headers | undefined) {
expect(new Map(otherHeaders)).toEqual(new Map(expectedHeaders));
}
it("should resolve HTTP Errors from XHR with headers", () => {
headers.set("Content-Type", "text/plain");
addHeaders(headers);
const err = parseErrorResponse({
...xhrHeaderMethods,
status: 500,
} as XMLHttpRequest) as HTTPError;
compareHeaders(headers, err.httpHeaders);
});
it("should resolve HTTP Errors from fetch with headers", () => {
headers.set("Content-Type", "text/plain");
addHeaders(headers);
const err = parseErrorResponse({
headers,
status: 500,
} as Response) as HTTPError;
compareHeaders(headers, err.httpHeaders);
});
it("should resolve Matrix Errors from XHR with headers", () => {
headers.set("Content-Type", "application/json");
addHeaders(headers);
const err = parseErrorResponse(
{
...xhrHeaderMethods,
status: 500,
} as XMLHttpRequest,
'{"errcode": "TEST"}',
) as MatrixError;
compareHeaders(headers, err.httpHeaders);
});
it("should resolve Matrix Errors from fetch with headers", () => {
headers.set("Content-Type", "application/json");
addHeaders(headers);
const err = parseErrorResponse(
{
headers,
status: 500,
} as Response,
'{"errcode": "TEST"}',
) as MatrixError;
compareHeaders(headers, err.httpHeaders);
});
});
it("should set a sensible default error message on MatrixError", () => {
let err = new MatrixError();
expect(err.message).toEqual("MatrixError: Unknown message");
@@ -188,14 +256,11 @@ describe("parseErrorResponse", () => {
});
it("should handle no type gracefully", () => {
// No Content-Type header
expect(
parseErrorResponse(
{
headers: {
get(name: string): string | null {
return null;
},
},
headers,
status: 500,
} as Response,
'{"errcode": "TEST"}',
@@ -203,31 +268,38 @@ describe("parseErrorResponse", () => {
).toStrictEqual(new HTTPError("Server returned 500 error", 500));
});
it("should handle invalid type gracefully", () => {
it("should handle empty type gracefully", () => {
headers.set("Content-Type", " ");
expect(
parseErrorResponse(
{
headers: {
get(name: string): string | null {
return name === "Content-Type" ? " " : null;
},
},
headers,
status: 500,
} as Response,
'{"errcode": "TEST"}',
),
).toStrictEqual(new Error("Error parsing Content-Type ' ': TypeError: invalid media type"));
).toStrictEqual(new Error("Error parsing Content-Type '': TypeError: argument string is required"));
});
it("should handle plaintext errors", () => {
it("should handle invalid type gracefully", () => {
headers.set("Content-Type", "unknown");
expect(
parseErrorResponse(
{
headers: {
get(name: string): string | null {
return name === "Content-Type" ? "text/plain" : null;
},
},
headers,
status: 500,
} as Response,
'{"errcode": "TEST"}',
),
).toStrictEqual(new Error("Error parsing Content-Type 'unknown': TypeError: invalid media type"));
});
it("should handle plaintext errors", () => {
headers.set("Content-Type", "text/plain");
expect(
parseErrorResponse(
{
headers,
status: 418,
} as Response,
"I'm a teapot",