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

lower case method? #58

Closed
bsdis opened this issue Oct 2, 2020 · 14 comments · Fixed by #71 or #66
Closed

lower case method? #58

bsdis opened this issue Oct 2, 2020 · 14 comments · Fixed by #71 or #66
Labels
discussion Long-term discussions and planning has fix A fix is PR'd or merged, but not yet released

Comments

@bsdis
Copy link

bsdis commented Oct 2, 2020

when i do redaxios.patch, i can see that its translated to patch and not PATCH which gives problems in terms of 400 bad request.

Looking through the redaxios code this seems intentional. Why? According to RFC 7231 page 21. 1) the table uses capital case and doesn't say anywhere that variations of the values in the table are allowed. 2) last sentence on page 20
methods are case sensitive and should be upper case.

Am I missing something here?

@bsdis bsdis changed the title lower case? lower case method? Oct 2, 2020
@developit
Copy link
Owner

Can you give an example where this produces an error? Lowercase HTTP method names are allowed by fetch() and should be transformed to uppercase prior to issuing the network request. Perhaps you're using a fetch polyfill that isn't doing this correctly?

@developit developit added the discussion Long-term discussions and planning label Oct 9, 2020
@bsdis
Copy link
Author

bsdis commented Oct 9, 2020

i used .patch method and got 404 bad request error. I dont think i used any polyfills...i just used redaxios out of the box. As soon as i switched back to normal axios.patch it worked great and where redaxios produced a patch request, axios produced a PATCH request and its been working since

@developit
Copy link
Owner

What server/service is producing the 400?

@bsdis
Copy link
Author

bsdis commented Oct 9, 2020

its a 404... the error already seemed to come before request even was transmitted to the server.
Almost asif the browser halted it even.
I am using a django backend

@developit
Copy link
Owner

tbh that really does sound like a polyfill then. Are you able to provide any details about your frontend setup? Are you using a build tool or off-the-shelf setup like Next.js, create-react-app, Vue CLI, etc?

@bsdis
Copy link
Author

bsdis commented Oct 9, 2020 via email

@jesec
Copy link

jesec commented Nov 11, 2020

I can confirm the bad request issue. My backend is Node.JS with Express.

browserslist (used by babel, postcss, etc.) is > 5% in my project. That means:

and_chr 86
chrome 86
chrome 85
ios_saf 14

I don't have any additional polyfill configuration in my project. Thus, it is extremely unlikely that the issue is related to polyfill.

Why not just simply use the upper case? It is more conforming and just makes more sense.

@bsdis
Copy link
Author

bsdis commented Nov 11, 2020

i have switched back to standard axios because of this and other suspicious behaviours

@ryands17
Copy link

@developit facing this issue as well. I have a repro with json-server with CRA here that faces this with patch.

Replacing axios with redaxios will highlight the issue.

@eperedo
Copy link

eperedo commented Apr 4, 2021

Same problem with nextjs v10.1.3 and redaxios 0.4.1 as unique dependency (besides react). According their docs polyfills are only included if browser does not support fetch.

Using patch method convert the method to lowercase, but using axios as a function works well.

// it works
await axios('/my-patch-endpoint', {
			method: 'PATCH',
			data: JSON.stringify({
				key: 'value',
			}),
		});

@Maggi64
Copy link

Maggi64 commented Apr 15, 2021

Encountered the same issue.
PATCH is case sensitive and needs to be uppercase: JakeChampion/fetch#254 (comment)

@semoal
Copy link

semoal commented Apr 16, 2021

Facing the same issue with json-server + latest vite version. Tried with axios and worked nicely, and then redaxios crashed about cors.

This is the error thrown
Screenshot 2021-04-16 at 20 18 58

@developit I can open a pull request with the line of "patch" to "PATCH", I've tried it locally and just changing that line works nicely.

@developit
Copy link
Owner

@semoal that would be good. Cc me when you do.

@semoal
Copy link

semoal commented Apr 16, 2021

@semoal that would be good. Cc me when you do.

Here you have Jason, #66 👍🏻

This was linked to pull requests Apr 29, 2022
@developit developit added the has fix A fix is PR'd or merged, but not yet released label Apr 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Long-term discussions and planning has fix A fix is PR'd or merged, but not yet released
Projects
None yet
7 participants