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

Array serialization truncation too aggressive #315

Closed
benvinegar opened this issue Jul 5, 2016 · 18 comments
Closed

Array serialization truncation too aggressive #315

benvinegar opened this issue Jul 5, 2016 · 18 comments

Comments

@benvinegar
Copy link

All arrays are currently being truncated to "Array of length N":

} elseif (is_array($value)) {

... and yet dictionaries are fully expanded. We are getting feedback from users with error data containing small arrays (e.g. 1-5 items) that are frustrated that they cannot see inside.

@lupoair
Copy link

lupoair commented Aug 1, 2016

Yes, frustrating!

@dcramer
Copy link
Member

dcramer commented Aug 1, 2016

I'm closing this out as we expand arrays in PHP.

@dcramer dcramer closed this as completed Aug 1, 2016
@idoberko2
Copy link

This issue still exists. Will it be fixed?

@dcramer
Copy link
Member

dcramer commented Aug 21, 2017

Just ton confirm, where are these arrays not getting expanded? (which section of the event)

@dcramer dcramer reopened this Aug 21, 2017
@idoberko2
Copy link

Additional Data. For example, we receive JSON webhooks from 3rd parties and send events to sentry in case of failure. Attached an example of how one of these JSONs look in Sentry:
image

@Jean85
Copy link
Contributor

Jean85 commented Aug 22, 2017

We should anyway limit the depth or let it be configurable, to avoid sending data too "deep".

@dcramer
Copy link
Member

dcramer commented Aug 22, 2017

I'm trying to recall why we went this direction in the first place. I think there were certain things where we didn't want the full data repr, but honestly dont recall where.

@rentalhost
Copy link

In my case, this additional data are required to make possible fix my issue, once that the problem is inside of this truncated array.

@pontus-mp
Copy link
Contributor

According to support this feature is intended to help keep error messages within the 100kb payload limit.

Maybe it would be possible to only flatten when the limit is hit, or make it opt-out? Those who are migrating from another solution (like us) are probably already having their error messages somewhat in check.

@Jean85
Copy link
Contributor

Jean85 commented Nov 13, 2017

In the 1.8.1 release that just got out, we added setters for the serializers. You can replace them with the logic you prefer.

@gsouf
Copy link

gsouf commented Nov 16, 2017

Hi @Jean85 can you give details about this method?

In my case the data truncated from $_SESSION are required to reproduce the error. To workarround the depth limit, I currently send a json encoded copy of $_SESSION (with pretty print) in extra param.

@Jean85
Copy link
Contributor

Jean85 commented Nov 16, 2017

I'm referencing this piece of code:

/**
* @param Raven_Serializer $serializer
*/
public function setSerializer(Raven_Serializer $serializer)
{
$this->serializer = $serializer;
}
/**
* @param Raven_ReprSerializer $reprSerializer
*/
public function setReprSerializer(Raven_ReprSerializer $reprSerializer)
{
$this->reprSerializer = $reprSerializer;
}

Those 2 serializers are the classes responsible for the compression of those arrays. You can extend them and change the behavior for the desired stuff. But using the extra field like that is a feasible solution too!

@gsouf
Copy link

gsouf commented Nov 16, 2017

Thanks @Jean85
Unfortunately setting the serializer was not useful in my case because the depth is set in Client::sanitize and wanted to set a greater depth for user data only.

I end up by ovewritting sanitize method to do the job. Here is an example in case it can help anyone:

class MyRavenClient extends \Raven_Client
{

    /**
     * @param array $data
     */
    public function sanitize(&$data)
    {
        // Serializes the session items as a json string
        foreach ($data['user']['data'] as $k => $v) {
            if (is_array($v)) {
                foreach ($v as $kk => $vv) {
                    $data['user']['data'][$k][$kk] = json_encode($vv, JSON_PRETTY_PRINT, 3);
                }
            }
        }


        // use parent sanitizer
        parent::sanitize($data);
    }

}

Be aware that serializer will limit the size of a string, and in this case it might be useful to provide an other serializer that has a greater limit on string size.

@Wirone
Copy link
Contributor

Wirone commented Oct 19, 2018

Also having this:

image

This relates to issues created via overriden Monolog's RavenHandler in Yii2 app (don't ask....).

I'll probably normalize trace to flat array, but still, this is frustrating.

@philraj
Copy link

philraj commented May 16, 2019

Any movement on this? It would be great if serialization depth was configurable for each of the types of data this client attaches to Sentry events (user, extra, tags...).

@Jean85
Copy link
Contributor

Jean85 commented May 16, 2019

Did you check out the new 2.0 release? The serializer has that option and it's completely replaceable if you want.

Also, for 1.x there's #632 which just needs to be released.

@philraj
Copy link

philraj commented May 16, 2019

@Jean85 that's great news, thank you. Will definitely be trying that out.

@stayallive
Copy link
Collaborator

This should be resolved in both 1.x and 2.x (but please use 2.x where possible).

Or well, the "culprit" is now configurable so if you run into this often it's possible to modify the settings for this behaviour.

1.x: #632
2.x: #378 (comment)

Briones pushed a commit to Briones/sentry-php that referenced this issue Feb 20, 2020
* Improve class_alias robustness

* Improve class_alias robustness

* ignore unavoidable PHPStan errors
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

No branches or pull requests