xini / silverstripe-mailchimp-signup

Adds page type for a MailChimp signup form. Form fields are read automatically from the MailChimp list.
BSD 3-Clause "New" or "Revised" License
4 stars 6 forks source link

throwing different error for spam #21

Closed sunnysideup closed 1 year ago

sunnysideup commented 1 year ago

I am looking here:

https://github.com/xini/silverstripe-mailchimp-signup/blob/fb1fa073d8a85f0e10ab1db209a50cbe10d08026/src/Extensions/SignupControllerExtension.php#L642-L644

I am getting lots of errors...


User Warning: Last Response: Array
(
    [headers] => Array
        (
            [url] => foo
            [content_type] => application/problem+json; charset=utf-8
            [http_code] => 400
            [header_size] => 355
            [request_size] => 337
            [filetime] => -1
            [ssl_verify_result] => 0
            [redirect_count] => 0
            [total_time] => 0.458694
            [namelookup_time] => 0.004227
            [connect_time] => 0.00545
            [pretransfer_time] => 0.014019
            [size_upload] => 197
            [size_download] => 229
            [speed_download] => 500
            [speed_upload] => 430
            [download_content_length] => 229
            [upload_content_length] => 197
            [starttransfer_time] => 0.014029
            [redirect_time] => 0
            [redirect_url] => 
            [primary_ip] => bar
            [certinfo] => Array
                (
                )

            [primary_port] => 443
            [local_ip] => foo
            [local_port] => foo bar
            [http_version] => 3
            [protocol] => 2
            [ssl_verifyresult] => 0
            [scheme] => HTTPS
            [appconnect_time_us] => 13880
            [connect_time_us] => 5450
            [namelookup_time_us] => 4227
            [pretransfer_time_us] => 14019
            [redirect_time_us] => 0
            [starttransfer_time_us] => 14029
            [total_time_us] => 458694
            [request_header] => foo
user-agent: DrewM/MailChimp-API/3.0 (github.com/drewm/mailchimp-api)
accept-encoding: deflate, gzip, br
accept: application/vnd.api+json
content-type: application/vnd.api+json
authorization: bar
content-length: 197

        )

    [httpHeaders] => Array
        (
            [server] => openresty
            [content-type] => application/problem+json; charset=utf-8
            [content-length] => 229
            [x-request-id] => foo
            [link] => <https://foo.api.mailchimp.com/schema/3.0/Definitions/ProblemDetailDocument.json>; rel="describedBy"
            [content-encoding] => gzip
            [vary] => Accept-Encoding
            [date] => Thu, 16 Mar 2023 20:48:54 GMT
        )

    [body] => {"type":"https://mailchimp.com/developer/marketing/docs/errors/","title":"Invalid Resource","status":400,"detail":"dodgy@yahoo.com has signed up to a lot of lists very recently; we're not allowing more signups for now","instance":"foo"}
)

It would be great to ignore these errors, because they are not errors, they are "success" in filtering spam.

sunnysideup commented 1 year ago

Here is what I did for a temporary fix:

if (strpos($mailChimp->getLastError(), 'has signed up to a lot of lists very recently') === false) {
xini commented 1 year ago

Hey Nicolaas, Well, that's debatable. The signup failed. Therefore it should show an error. Also, when you look at https://mailchimp.com/developer/marketing/docs/errors/#error-glossary, there is no way of telling what error is a filtering success and what is some other error. What do you think? Cheers

xini commented 1 year ago

Here is what I did for a temporary fix:

if (strpos($mailChimp->getLastError(), 'has signed up to a lot of lists very recently') === false) {

And then? Do we tell them that signup suceeded? That's not true. Also, what other "errors" like this exist? Also, how reliable is that the text in that error message stays the same?

sunnysideup commented 1 year ago
Well, that's debatable. The signup failed. Therefore it should show an error.

Yes debatable. Here is what I would say in the debate: I am getting 200+ errors from a site about people trying to sign-up. The code - mailchimp - is written in such a way that it blocks spammers. Blocking spammers is not an error, it is a success.

And then? Do we tell them that signup succeeded? That's not true.

Right now, the user also does not know as a E_USER_WARNING does throw an error on production? (not sure). image

Also, what other "errors" like this exist?
Also, how reliable is that the text in that error message stays the same?

Yes - obviously this is a hack. I can see it is a bit risky (but not that risky).

Right now, I am getting lots of errors each day, which makes me miss real errors.

xini commented 1 year ago

sorry, I think I now understand: you want to not throw an error in these cases, but still show the signup error to the user?

sunnysideup commented 1 year ago

Yes - I think that would make sense.

xini commented 1 year ago

ok. and I'll replace the user_error with Injector::inst()->get(LoggerInterface::class)->warning('...');?

sunnysideup commented 1 year ago

That sounds great.

xini commented 1 year ago

This is now tagged as 4.8.0 and 5.1.0. Thanks, Nicolaas!

sunnysideup commented 1 year ago

THANK YOU!!!!