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

Added Support for Msg91 phone provider #989

Closed
wants to merge 10 commits into from

Conversation

MeetChaudhari
Copy link

What kind of change does this PR introduce?

This PR adds support for Msg91.com , One of the best highly reliable Indian SMS provider rates even lesser than textlocal.

What is the current behavior?

Actually currently present textlocal integration for Indian audience highly unreliable not from the integration point, its from the textlocal servers and my company is currently developing an huge artist centric platform using supabase during our testing we had lot of issues due to textlocal. So today i have added Msg91 support

Additional context

The implementation is loosely based on textlocal implementation.
And due to Indian Government mandate, the message template, header has to be registered before sending any sms.

Also my implementation takes 3 params

  • Msg91 authkey
  • DLT template Id
  • Sender Id

For above this i will add documentation for obtain each of the params

@MeetChaudhari MeetChaudhari requested a review from a team as a code owner March 7, 2023 07:22
@kangmingtay
Copy link
Member

Hey @MeetChaudhari MeetChaudhari, thanks for the contribution! Unfortunately, i don't think we plan to merge this in since the team has decided to go with webhooks in the future for custom sms providers which would be more scalable in terms of maintaining the codebase. However, if you need this feature urgently, we recommend that you fork the codebase to include your changes in it.

@MeetChaudhari
Copy link
Author

@kangmingtay, I understand and agree with your decision, but the thing is, we do not want to self-host the platform, as such we have a lot of microservices to deal with and as founder, I need a bit of stability in the signup process that's the reason I added this like if it is possible to please help me out I would be very grateful to your team and whenever you guys switch to webhooks I'll do all the necessary changes required for my contribution.

Like till you guys implement the webhook solution, it would very helpful for me to get started for me with my pr.

@kiwicopple
Copy link
Member

Hey @MeetChaudhari - thanks for the additional context.

it's a tricky balance we need to strike between accepting a lot of PRs now (delaying the webhooks implementation) and commiting to the webhook approach.

I understand that textlocal might not be as cheap as Msg91, but it's already implemented and available. Is there any reason that you can't use it, beyond cost?

@MeetChaudhari
Copy link
Author

hey @kiwicopple , actually it's not about the cost it is about the reliability as a textlocal customer I have already purchased a good number of sms credits on their platform the problem is sometimes it works flawlessly sometimes it just gives random errors on very frequent basis and this is not an issue with the supabase textlocal integration authored by devkiran, it's from textlocal end. I even had a discussion with devkiran about the issue with textlocal.

And just think if I want to get my dream project into production I seriously need to have the reliability of a service.

@kangmingtay
Copy link
Member

Hey @MeetChaudhari, if cost is not an issue and you're looking for reliability, will it be possible to use one of the other providers instead? We also support using Twilio, Messagebird and Vonage on Supabase. Is there a particular reason why you are choosing Msg91 over these other providers?

@MeetChaudhari
Copy link
Author

Cause that becomes comparatively too expensive for us, as our platform is situated in India for comparison

This are rates for sending one message in India in USD. (All prices are taken from their respective brand sites.)

  • Vonage $0.0436

  • MessageBird $0.0420

  • Twilio $0.0490

  • TextLocal. $0.0036 29.5 paisa (INR)

  • Msg91 $0.0030 25.0 paisa (INR)

so its pretty obvious for us to use Indian providers

@kiwicopple
Copy link
Member

problem is sometimes it works flawlessly sometimes it just gives random errors on very frequent basis

I guess the challenge for us here - how do we know that it's not just the same problem with msg91? From a quick search, there are quite a few SMS providers in India. If we keep merging new providers and each of them is buggy, it creates more delays for the Auth team to implement the "right" solution (webhooks).

Have you raised a ticket with the textlocal team to help solve any issues?

@MeetChaudhari
Copy link
Author

Hey @kiwicopple i agree with you, the problem was i tried everything possible even my textlocal logs, i agree did not raise any tickets with textlocal team, but not in case of this integration, i signed up as startup on msg91, i have prior experience with msg91, this company serves for multiple unicorn companies in india and my prior experience is flawless with them, supabase textlocal Integration made me switch to textlocal.

And as i signed up as startup which is what i have, i have a contact/ direct support of developer form msg91 to mitigate any issues, like even while making integration i had some doubts which was instantly solved by msg91 team.

@MeetChaudhari
Copy link
Author

https://www.fonada.com/blog/top-10-bulk-sms-service-providers-in-india/

Check this recent article. For more reference

@kangmingtay
Copy link
Member

@MeetChaudhari We see where you're coming from but we are still hesitant to push forward on this for now because providing top-notch support for this feature could be challenging given the issues we've experienced with sending SMSes from other service providers (TextLocal). For example, it was impossible for our support team to debug issues with TextLocal since there's a requirement for one to own a SIM card from India, which requires one to be a citizen of India. Additionally, if we merge in this PR, it could make it harder to turn down future SMS providers since there may always be a case for another one being more reliable or cost-effective. We also haven't received any other request to add Msg91 as an SMS provider and we don't add integrations based on one-off requests. For example, we keep a list of external OAuth provider requests here and we only accept contributions for those that are deemed as "in-demand" from our community.

Our goal is to make gotrue more flexible by introducing features like webhooks, which will allow developers to integrate any third-party software they choose, instead of building a customised solution for every auth flow.

One alternative that you can consider here will be to host a fork of GoTrue on providers like Fly, which have a free tier, until we've worked out the webhook implementation.

@MeetChaudhari
Copy link
Author

Yes i completely understand, but as i have talked with msg91 team personally for your problems, they have responded me this

"We can even help their team in testing and resolving any bugs and I can personally be in touch with them for that.
They will not have to get an indian number to debug and do the testing, we'll assist them in that dedicatedly."

And i will take care of documentation and maintenance of the integration till you guys implement the webhook solution with being in direct contact with msg91 team. They will help your team to test out integration perfectly. As mentioned above.

MSG91 team provides three channels for support to all their customers -

  1. Email - [email protected]
  2. Phone - +918818888733
  3. Chat - Available on Website

Specifically for Supabase team, they have agreed to assign a dedicated partner manager which can
help you in debugging all issues pertaining to this integration. Details of the Partner Manager is mentioned below along with their direct email and number.

Hardik Menghani
[email protected] / [email protected]
+918889500122

Hardik can help you in all challenges - may it be with regards to tech/business/regulation.
They can also assist you in debugging issues without getting an Indian Mobile number and for any other queries which may arise in the future.

No other SMS provider in India can offer you services at such a cost-effective pricing with multi-channel support like what MSG91 is offering.

.github/workflows/test-m.yml Outdated Show resolved Hide resolved
internal/api/sms_provider/msg91.go Outdated Show resolved Hide resolved
internal/api/sms_provider/msg91.go Outdated Show resolved Hide resolved
internal/api/sms_provider/msg91.go Outdated Show resolved Hide resolved
internal/api/sms_provider/msg91.go Show resolved Hide resolved
internal/api/sms_provider/msg91.go Show resolved Hide resolved
internal/api/sms_provider/msg91.go Outdated Show resolved Hide resolved
internal/api/sms_provider/msg91.go Outdated Show resolved Hide resolved
internal/api/sms_provider/msg91.go Outdated Show resolved Hide resolved
internal/conf/configuration.go Outdated Show resolved Hide resolved
.github/workflows/test-m.yml Outdated Show resolved Hide resolved
example.docker.env Outdated Show resolved Hide resolved
example.env Show resolved Hide resolved
example.env Outdated Show resolved Hide resolved
@J0
Copy link
Contributor

J0 commented Mar 20, 2023

Thanks for the work on the PR thus far! Gentle reminder to file a PR to the phone-login section of our documentation with setup instructions and link back to this PR so that we can try to test the integration. It'll also be really helpful in helping other developers figure out how to use the integration. Feel free to reference twilio.mdx as an example

@MeetChaudhari MeetChaudhari requested review from kangmingtay and hf and removed request for kangmingtay March 22, 2023 11:37
@MeetChaudhari MeetChaudhari requested review from kangmingtay and removed request for hf March 22, 2023 18:34
@MeetChaudhari MeetChaudhari requested a review from hf April 1, 2023 12:45
Copy link
Contributor

@hf hf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is generally good now @MeetChaudhari, @kangmingtay, @J0. I'll try to fix some of the technical comments and merge it.

@MeetChaudhari
Copy link
Author

any update on this?

@hf
Copy link
Contributor

hf commented Jul 4, 2023

@MeetChaudhari I've forked this #1018 and fixed it up. I'll close this PR as it's quite out of date.

Do you think it's possible for you to contribute some docs for Msg91 similar to these here: https://supabase.com/docs/guides/auth/phone-login/twilio

@hf hf closed this Jul 4, 2023
@MeetChaudhari
Copy link
Author

@MeetChaudhari I've forked this #1018 and fixed it up. I'll close this PR as it's quite out of date.

Do you think it's possible for you to contribute some docs for Msg91 similar to these here: https://supabase.com/docs/guides/auth/phone-login/twilio

Yes i can do it, actually I was waiting for pr to get merged

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.

5 participants