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

Library should throw an exception for an email with no subject #76

Closed
nquinlan opened this issue Mar 7, 2014 · 5 comments
Closed

Library should throw an exception for an email with no subject #76

nquinlan opened this issue Mar 7, 2014 · 5 comments

Comments

@nquinlan
Copy link
Contributor

@nquinlan nquinlan commented Mar 7, 2014

@iansltx: @sendgrid Y'all's PHP API should throw an InvalidArgumentException or similar if, via HTTP, an email has no subject. Just sayin'.

https://twitter.com/iansltx/status/441794697669787648

@iansltx
Copy link

@iansltx iansltx commented Mar 7, 2014

Further background: when e-mails are sent via SMTP, no subject (or a subject of '') is acceptable, generates no errors, and sends just fine. However when using the web API, $mail->web->send() with a zero-length subject returns an error response. Since other issues with API interaction throw exceptions, this should probably do that as well, since otherwise the transactions more or less fail silently.

@eddiezane
Copy link
Contributor

@eddiezane eddiezane commented Oct 30, 2014

Hey @iansltx buddy, long time no see :). I've had a go at this and have decided it's best aligned with our other libraries to throw an error if the response code is not 200.

    if ($response->code != 200) {
      throw new SendGrid\Exception($response->raw_body);
    }

My question for you is, as a PHP developer, would you rather see the raw body raised

SendGrid\Exception: {"message": "error", "errors": ["Bad username / password"]}

Or a parsed object?

stdClass Object
(
    [message] => error
    [errors] => Array
        (
            [0] => Bad username / password
        )

)

Can please weigh in too @kunal732?

@nquinlan
Copy link
Contributor Author

@nquinlan nquinlan commented Oct 30, 2014

I'm for parsed object.

@iansltx
Copy link

@iansltx iansltx commented Oct 30, 2014

The default \Exception class doesn't have the concept of a non-string message. You can override it, but being able to getMessage() on SendGrid\Exception and get a string...the unparsed response body...back would be the right thing to do here.

That said, SendGrid\Exception can extend the normal exception class with something like:

public function getErrors() {
    return json_decode($this->message)['errors'];
}

...that's where you'd put the parsed object bits.

eddiezane added a commit that referenced this issue Nov 6, 2014
Closes #76. This will require a major version bump
@eddiezane
Copy link
Contributor

@eddiezane eddiezane commented Nov 7, 2014

Thanks for the help buddy! I'm slowly picking PHP up. Will be merged when I close some other issues and release v3.0.0.

@eddiezane eddiezane closed this Nov 7, 2014
martijnmelchers pushed a commit to martijnmelchers/sendgrid-php that referenced this issue Apr 9, 2018
martijnmelchers pushed a commit to martijnmelchers/sendgrid-php that referenced this issue Apr 9, 2018
Added Pull Requests Review section to CONTRIBUTE.md Closes sendgrid#76
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.