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

fix: return early from sectorPathUpdate if path is invalid #3047

Merged
merged 1 commit into from
Nov 27, 2020
Merged

fix: return early from sectorPathUpdate if path is invalid #3047

merged 1 commit into from
Nov 27, 2020

Conversation

fnune
Copy link
Contributor

@fnune fnune commented Nov 26, 2020

Checklist
  • npm test passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Description of change

In a project, I'm getting Cannot read property '0' of undefined. Sentry report:

Crashed in non-app:
/tmp/build_dfe7ac4f_/node_modules/@antv/g2/esm/animate/animation/sector-path-update.js in getArcStartPoint

This is (superficially) because there's nothing that checks whether toAttrs.path is a valid PathCommand[]. I've added that check with an early return.

@github-actions
Copy link

github-actions bot commented Nov 26, 2020

😭 Deploy PR Preview d2119c7 failed. Build logs

🤖 By surge-preview

@coveralls
Copy link

coveralls commented Nov 26, 2020

Pull Request Test Coverage Report for Build 385643529

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.05%) to 93.423%

Files with Coverage Reduction New Missed Lines %
src/animate/animation/sector-path-update.ts 1 93.07%
Totals Coverage Status
Change from base Build 384746160: 0.05%
Covered Lines: 10541
Relevant Lines: 10940

💛 - Coveralls

@hustcc hustcc merged commit 694f209 into antvis:master Nov 27, 2020
@hustcc
Copy link
Member

hustcc commented Nov 27, 2020

Thanks

@fnune
Copy link
Contributor Author

fnune commented Nov 27, 2020

No, thank you!

@fnune fnune deleted the return-defensively-sector-path-update branch November 27, 2020 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants