`sys.argv` and `command` are sent to sentry without the last character of each item

[yipit]

`sys.argv` and `command` are sent to sentry without the last character of each item

[tltx]

Hi, are you going to accept my pull request or do you see another solution?

[hltbra]

Sorry, @tltx, I missed github's email notification. I will comment on your PR.

[ralish]

The cause of this bug is due to the meaning of -1 as it applies to Python's slice notation. By setting the string_max_length parameter to -1 it's interpreted by some of Raven's serializers to mean "the string minus the last character". Take a look at the highlighted serializers in raven/utils/serializer/base.py.

In short, there's no sane way that I can see to specify no limit for the string data. This is I think generally fair, as the Sentry server is always going to impose its limits, and the client has good reason to impose its own either in recognition of the upstream server limits or for its own reasons.

Taking the current Sentry sources as an example, SENTRY_MAX_EXTRA_VARIABLE_SIZE is set to 4096, and so each key's value in the extra dictionary will be trimmed to this value regardless, up to a maximum number of keys defined by SENTRY_MAX_DICTIONARY_ITEMS, which defaults to 50. As for Raven, string_max_length defaults to MAX_LENGTH_STRING which is 400 (though the documentation doesn't seem to reflect this).

My suggestion would be to set the limit to 4096 to reflect the current Sentry default, in recognition that anything higher than this will require overriding the default in Sentry's conf/server.py. A comment should be added to indicate this to help eliminate confusion as to why keys in the extra data structure with values larger than this limit are being silently truncated.

Happy to create a pull request to implement the above change if this is agreeable.

[tltx]

This is what I tried to do in my PR. Setting the default of string_max_length to 4096 and adding a comment about the Sentry default is a good idea. Please make a PR, this is how it should be.

[hltbra]

@ralish, how is your suggestion different from @tltx's pull request #14?

[hltbra]

I am eager to merge #14 and release a new version, but I would like to understand what's missing or wrong from your understanding, @ralish.

[tltx]

As far as I can tell SENTRY_MAX_EXTRA_VARIABLE_SIZE in the Sentry config limits the total number of chars in strings in the extra dict, that cron-sentry use. I have updated my PR to reflect this.

[tltx]

@hltbra do you have any feedback on the PR or is it time to merge it?

[ralish]

Apologies for never replying to this! The notification got lost in a mail deluge. Good result regardless :)