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

Accept HTTP 201 response codes #686

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

rharriso
Copy link

@rharriso rharriso commented Jan 16, 2023

What does this PR do?

Accept HTTP response code 201. See issue #685

Checklist

  • All new functionality has tests.
  • All tests are passing.
  • New or changed API methods have been documented.
  • npm run format has been run

CHANGELOG

  • [FIXED] Support response code 201 from successful POST requests

@stale
Copy link

stale bot commented Apr 17, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you'd like this issue to stay open please leave a comment indicating how this issue is affecting you. Thank you.

@stale stale bot added the wontfix label Apr 17, 2023
@stale stale bot closed this Apr 25, 2023
@fbenevides fbenevides reopened this May 31, 2023
@stale stale bot removed the wontfix label May 31, 2023
@fbenevides fbenevides mentioned this pull request Jun 12, 2023
4 tasks
@amad
Copy link
Contributor

amad commented Jun 12, 2023

@rharriso pusher.authorizeChannel shouldn't result a 201. I couldn't reproduce it. Could you please share a code snippet?

If you are using an open source web application framework, please share its name and version.

@rharriso
Copy link
Author

Not supporting 201 is a defect. It's not necessary to enumerate all of the frameworks that use 201 for successful authentication.

  • 201 Created is a more specific success response than simply "OK".
  • It is very common for frameworks or libraries to define logins as "creating" a session or token or whatever is being used to say that a given user is logged in right now.

Requiring the response code to be 200 necessitated an adapter in the system I was working on 6 months ago. I can't remember if I had to intercept the response on the server or the client, and I no longer have access to that project so I can't confirm.

I'm no longer working with PusherJS, so it doesn't matter to me if this defect is fixed, but it is a defect.

@amad
Copy link
Contributor

amad commented Jun 12, 2023

I appreciate your comment.

I'll investigate a little deeper to see if I can find an example to share with the team.

My initial theory was that perhaps your app was registering the user (i.e., creating a new resource) with the first auth request, and that was the reason for the web framework to return a 201 status code.

@amad amad self-assigned this Jun 12, 2023
@KayakinKoder
Copy link

Hasura is an example of a service which returns a 201 (for pretty much all successful api response use cases).

Pusher not allowing the 201 was one of those multi-hour head scratchers that hopefully future users won't have to go through.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants