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

truncate crash log string to 60000 characters #84

Merged
merged 2 commits into from
Jul 11, 2017

Conversation

juergenm
Copy link

@juergenm juergenm commented Jul 6, 2017

to fix "Request body to large" error. fixes #82

// chrash reports seems to be limited to 64kb, otherwise they are ignored by the API with the following error
// [Airbrake] {"code":400,"type":"Bad Request","message":"http: request body too large"}
// so we truncate the string to 60000 characters
if(dataStr.length > 60000) {
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Our API limit is 64KB, not 60000 characters, hence this is not the most precise check
  2. Looks like you truncate a string. We need to check dumped JSON if it's bigger than 64KB and then truncate payload in the notice object

@juergenm
Copy link
Author

juergenm commented Jul 7, 2017

@kyrylo Thats true this check was not very precise. I now first create the JSON, check the size and if it is too large i truncate the payload string

@kyrylo kyrylo requested a review from jocelynlih July 7, 2017 14:32
@jocelynlih
Copy link
Contributor

This looks good to me. My only concern is that the size limit is hardcoded in the API.
But I suppose it won't get updated/changed unless a new API version is used? @kyrylo

@kyrylo
Copy link
Contributor

kyrylo commented Jul 11, 2017

@jocelynlih that's a correct assumption. We hardcode this in every notifier that supports this feature.

Copy link
Contributor

@jocelynlih jocelynlih left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@kyrylo kyrylo merged commit 6fc4456 into airbrake:master Jul 11, 2017
@kyrylo
Copy link
Contributor

kyrylo commented Jul 11, 2017

Thanks, @juergenm!

@juergenm juergenm deleted the truncate_crash_report branch July 12, 2017 03:56
@juergenm
Copy link
Author

@kyrylo thanks for merging, could you please release a new version so that I can use it with CocoaPods.

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.

Notice not posted successfully because "Request body too large"
3 participants