Skip to content
This repository has been archived by the owner on Oct 23, 2023. It is now read-only.

Cleaning up wrap, adding intercept #225

Merged
merged 4 commits into from
Nov 1, 2016
Merged

Cleaning up wrap, adding intercept #225

merged 4 commits into from
Nov 1, 2016

Conversation

LewisJEllis
Copy link
Contributor

This is a little WIPpy, need to test more, but I realized 1) usefulness of an intercept function and 2) some simplifications to wrap.

@LewisJEllis
Copy link
Contributor Author

LewisJEllis commented Nov 1, 2016

@benvinegar or @MaxBittker take a look?

Main consideration - is intercept the right name for this; was originally inspired by domain.intercept but this isn't really using domains, just an errback pattern; also thinking about interceptErr or wrapErr or something.

Usage pattern is:

doSomethingThatTakesCallback(Raven.intercept(function (err, result) {
  // err will never be instanceof Error; if it was, intercept caught it, reported, didn't call this cb
  // can proceed without patterns like below:
  if (err) return next(err);
  if (err) return Raven.captureException(err);
});

Another consideration - callback could really just be function (result) since we know err will never be passed, but I think we should keep the same signature so this can just be used as a wrapper around callbacks that already expect the function (err, callback) pattern.

@MaxBittker
Copy link
Contributor

re: naming, the special thing about this intercept function is that it works with the node err callback argument convention, vs the exception world (which doesn't work with async callbacks). That being said, would a name that references this is specifically for callback errs be more descriptive?

@MaxBittker
Copy link
Contributor

the code lgtm

Copy link
Contributor

@MaxBittker MaxBittker left a comment

Choose a reason for hiding this comment

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

intercept looks good to me, wrapdomain change seems to make sense 👍

Copy link
Contributor

@MaxBittker MaxBittker left a comment

Choose a reason for hiding this comment

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

lgtm

@LewisJEllis LewisJEllis merged commit dbec003 into master Nov 1, 2016
@LewisJEllis LewisJEllis deleted the intercept branch November 1, 2016 22:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants