Skip to content

dataCallback, shouldSendCallback are passed prior callback as 2nd arg - #636

Merged
benvinegar merged 1 commit into
masterfrom
get-exist-callbacks
Jul 1, 2016
Merged

benvinegar merged 1 commit into
masterfrom
get-exist-callbacks

Conversation

@benvinegar

@benvinegar benvinegar commented Jun 30, 2016 •

Copy link
Copy Markdown
Contributor

Fixes #428 (and eventually #477)

Now you can do the following for both setDataCallback and setShouldSendCallback:

Raven.setDataCallback(function (data, priorDataCallback) {
  if (priorDataCallback) {
    data = priorDataCallback(data);
  }
  data.message = 'derp';
  return data;
});

This way it is possible to preserve OR override the behavior of any previously set callback, without adding new API methods (e.g. addDataCallback and addShouldSendCallback). It also lets subsequent callbacks mutate data before or after the prior callback is invoked, which the add API is not capable of doing (execution would be serial FIFO).

I'm not 100% on doing this, but I think it could be a satisfactory solution – especially since we've gone this far without it. Would love to get the community's feedback.

cc @mattrobenolt @nevir @mitsuhiko

@nevir

nevir commented Jul 1, 2016 •

Copy link
Copy Markdown
Contributor

This way it is possible to preserve OR override the behavior of any previously set callback, without adding new API methods (e.g. addDataCallback and addShouldSendCallback).

Yay! Much better

I'm not 100% on doing this, but I think it could be a satisfactory solution – especially since we've gone this far without it. Would love to get the community's feedback.

It would definitely cover my needs for #626

@mattrobenolt

Copy link
Copy Markdown
Contributor

🍪

@mitsuhiko

Copy link
Copy Markdown
Contributor

🍪

@benvinegar
benvinegar merged commit cb86145 into master Jul 1, 2016
@benvinegar
benvinegar deleted the get-exist-callbacks branch July 1, 2016 22:18
pull Bot pushed a commit to TheDegenerateDev5150/sentry-javascript that referenced this pull request Jun 19, 2026
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.

4 participants