Skip to content

feat: Allow to pass errorHandler as record option#1107

Merged
YunFeng0817 merged 3 commits intorrweb-io:masterfrom
getsentry:fn/errorHandler
Mar 22, 2023
Merged

feat: Allow to pass errorHandler as record option#1107
YunFeng0817 merged 3 commits intorrweb-io:masterfrom
getsentry:fn/errorHandler

Conversation

@mydea
Copy link
Contributor

@mydea mydea commented Feb 3, 2023

We've been playing around quite a lot with ways to handle errors that originate somewhere from within rrwebs deep callback stack. This PR adds an option to add a errorHandler option to record, which can be used to either swallow errors or handle them in different ways:

record({
  errorHandler: (error) => {
    // return `true` to swallow the error and stop propagating
    // Or log the error somewhere, etc.
  }
});

I tried to wrap all places that are called as (async) callbacks somewhere. I may have missed a place, but IMHO this should give a pretty decent coverage.

Note that in order to do that, I had to rewrite the monkey patching we are doing on e.g. CSSStyleSheet to use a proxy. Otherwise, retaining the this context correctly turned out quite tricky. IMHO this is a nicer approach anyhow, and all supported browsers (> IE11) support proxies, so should be OK I think.

@mydea mydea mentioned this pull request Feb 3, 2023
Merged
@YunFeng0817 YunFeng0817 self-requested a review February 7, 2023 07:40
@Lms24 Lms24 mentioned this pull request Feb 7, 2023
Closed
Juice10
Juice10 reviewed Feb 9, 2023
View reviewed changes
YunFeng0817
YunFeng0817 approved these changes Mar 19, 2023
View reviewed changes
Copy link
Member

@YunFeng0817 YunFeng0817 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I feel like this is a good mechanism. Using "Proxy" constructor will make the browser compatibility worse. Otherwise, it's a good way to do patching things.
Could you please solve the merge conflicts and add a change log for this one?

@mydea mydea force-pushed the fn/errorHandler branch from a2d434a to 12978c0 Compare March 20, 2023 12:01
@changeset-bot
Copy link

changeset-bot bot commented Mar 20, 2023 •
edited
Loading

🦋 Changeset detected

Latest commit: 4144e08

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages
Name Type
rrweb Minor
rrweb-snapshot Minor
rrdom Minor
rrdom-nodejs Minor
rrweb-player Minor
@rrweb/types Minor
@rrweb/web-extension Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@mydea
Copy link
Contributor Author

mydea commented Mar 20, 2023

I resolved the merge commits and added a changeset!

@YunFeng0817
Copy link
Member

YunFeng0817 commented Mar 20, 2023

Sorry, one more thing before merging this. Could you also add the description of this config in the rrweb doc?

@mydea mydea force-pushed the fn/errorHandler branch from 62f408f to 4befd4b Compare March 20, 2023 12:31
@mydea
Copy link
Contributor Author

mydea commented Mar 20, 2023

Sorry, one more thing before merging this. Could you also add the description of this config in the rrweb doc?

Ah sure, of course! I added this to the guide doc as well.

@YunFeng0817
Copy link
Member

YunFeng0817 commented Mar 20, 2023

Thank you!
Seems like the CI keeps failing because of this lint error:
image

Could you help me to add this line to the ZH-CN doc:

| errorHandler             | -                  | 一个可以定制化处理错误的毁掉函数，它的参数是错误对象。如果 rrweb recorder 内部的某些内容抛出错误，则会调用该回调。                                                                    |

@YunFeng0817 YunFeng0817 merged commit a225d8e into rrweb-io:master Mar 22, 2023
@github-actions github-actions bot mentioned this pull request Mar 22, 2023
Merged
@mydea
Copy link
Contributor Author

mydea commented Mar 22, 2023

Thank you! Seems like the CI keeps failing because of this lint error: image

Could you help me to add this line to the ZH-CN doc:

| errorHandler             | -                  | 一个可以定制化处理错误的毁掉函数，它的参数是错误对象。如果 rrweb recorder 内部的某些内容抛出错误，则会调用该回调。                                                                    |

Ah, is this now resolved already? Or anything else I should do?

@YunFeng0817
Copy link
Member

YunFeng0817 commented Mar 22, 2023

Yes, I resolved it in another PR.

@mydea mydea deleted the fn/errorHandler branch March 22, 2023 13:41
@github-actions github-actions bot mentioned this pull request Mar 23, 2023
Open
This was referenced Jul 6, 2023
Closed
Closed
Closed
@billyvg billyvg mentioned this pull request Jul 17, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Juice10 Juice10 Juice10 left review comments

@YunFeng0817 YunFeng0817 YunFeng0817 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mydea @YunFeng0817 @Juice10