Change constructor signature #3
Milestone
Comments
|
In theory, I suppose the API URL could also be a request option, but that feels strange to me. I think we’ll keep that as the first constructor argument. |
lucaswerkmeister
added a commit
that referenced
this issue
Sep 7, 2024
This partially walks back f6e28b5. It turns out that the type of the fetch body affects the Content-Type sent with the request: URLSearchParams corresponds to application/x-www-form-urlencoded and FormData to multipart/form-data. For a simple request, the former looks like this: Content-Type: application/x-www-form-urlencoded;charset=UTF-8 meta=siteinfo&format=json Whereas the latter looks like this: Content-Type: multipart/form-data; boundary=---------------------------384998708438653208232613793789 -----------------------------384998708438653208232613793789 Content-Disposition: form-data; name="meta" siteinfo -----------------------------384998708438653208232613793789 Content-Disposition: form-data; name="format" json -----------------------------384998708438653208232613793789-- So using URLSearchParams is already much more efficient, resulting in fewer bytes being sent over the wire (though in most m3api applications the majority of traffic will be received from the server anyway; and note that this only applies to POST requests anyway, not GET requests). Additionally, it turns out that OAuth 2.0 requires requests to the access token endpoint to be sent using application/x-www-form-urlencoded (according to section 4.1.3 of RFC 6749 [1]), and recently MediaWiki started to enforce this [2], which meant that m3api-oauth2 was broken for Wikimedia production with all m3api versions since 0.8.0. Using URLSearchParams when possible fixes lucaswerkmeister/m3api-oauth2/#3. [1]: https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.3 [2]: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/OAuth/+/1046715
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The signature of
request()is nice and tidy at the moment: one object of params (sent to the API), and an optional object of options (controlling how the params are sent to the API: method, max retries, etc.).The constructor is a bit more messy: it has the API URL, then an object of default params, and then the user agent, which logically belongs with the request options. There’s a hard split between the user agent and all the other request options: you can’t change the user agent for just one
request()call, nor can you set any default request options in the constructor.We probably want to change this, and we probably want to do that pre-1.0.0. The constructor should take the API URL, the default params, and then the default options; the user agent would be an option like any other.
The text was updated successfully, but these errors were encountered: