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

fix: set mediaType to null if contentType cannot be parsed #911

Merged
merged 2 commits into from Dec 10, 2019

Conversation

jmorrise
Copy link
Contributor

@jmorrise jmorrise commented Dec 10, 2019

Fixes issue #905 by setting mediaType to null if the contentType is invalid.

@jmorrise jmorrise requested a review from as a code owner Dec 10, 2019
@googlebot googlebot added the cla: yes label Dec 10, 2019
Avoids users hitting a runtime exception when the contentType is
invalid.
@jmorrise
Copy link
Contributor Author

@jmorrise jmorrise commented Dec 10, 2019

I force-pushed to resolve the conventionalcommits error, but I'm afraid things might be in a weird state now - let me know if I need to fix this.

@chingor13 chingor13 changed the title Set mediaType to null if contentType cannot be parsed. fix: set mediaType to null if contentType cannot be parsed. Dec 10, 2019
@chingor13 chingor13 changed the title fix: set mediaType to null if contentType cannot be parsed. fix: set mediaType to null if contentType cannot be parsed Dec 10, 2019
@chingor13 chingor13 added the kokoro:force-run label Dec 10, 2019
@kokoro-team kokoro-team removed the kokoro:force-run label Dec 10, 2019
@chingor13
Copy link
Collaborator

@chingor13 chingor13 commented Dec 10, 2019

Note that this is potentially a behavior breaking change in that previously you could get an InvalidArgumentException (unchecked, runtime exception) when executing a request. Now, you will get the response with a null (unparsed) MediaType which would seem to be always preferred anyways.

@chingor13 chingor13 merged commit 7ea53eb into googleapis:master Dec 10, 2019
9 checks passed
Copy link
Collaborator

@elharo elharo left a comment

There might be an argument for defaulting to application/octet-stream instead of null.

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

Successfully merging this pull request may close these issues.

None yet

7 participants