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.

Sign up for GitHub

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

Jump to bottom

unhelpful error message when taking screenshot of element with zero height #5149

Closed
egrubbs opened this issue Sep 16, 2019 · 3 comments · Fixed by #5078
Closed

unhelpful error message when taking screenshot of element with zero height #5149

egrubbs opened this issue Sep 16, 2019 · 3 comments · Fixed by #5078
Labels
pkg/driver This is due to an issue in the packages/driver directory topic: screenshots 📸 type: bug

Comments

@egrubbs
Copy link

egrubbs commented Sep 16, 2019 •
edited by jennifer-shehane
Loading

Current behavior:

cy.get(element).screenshot() displays a very confusing error if the element has zero height.

TypeError: Cannot destructure property `duration` of 'undefined' or 'null'.

image

The code that is causing the confusing error

numScreenshots ends up being zero when elPosition.height is zero.

cypress/packages/driver/src/cy/commands/screenshot.coffee

Line 138 in c096251

numScreenshots = Math.ceil(elPosition.height / viewportHeight)

This causes _.last(results) to return undefined.

cypress/packages/driver/src/cy/commands/screenshot.coffee

Line 99 in c096251

_.last(results)

Which causes takeScreenshot().then() to receive an undefined props parameter that it cannot destructure with { duration, path, size } = props;.

cypress/packages/driver/src/cy/commands/screenshot.coffee

Line 357 in c096251

{ duration, path, size } = props

Desired behavior:

cy.get(element).screenshot() should report an error that the element has zero height.

It is also extremely difficult to troubleshoot an error like this because bluebirdjs doesn't automatically log exceptions, so I have no idea where the code in the error message existed.

It appears that bluebirdjs has events for unhandled errors. It would be great if Cypress provided a filename and line number of the where the error is occurring in the Cypress log as well as a stack trace in the browser console.

http://bluebirdjs.com/docs/api/error-management-configuration.html

process.on("unhandledRejection", function(reason, promise) {
});
process.on("rejectionHandled", function(promise) {
});

Steps to reproduce: (app code and test code)

You can recreate the confusing error message by trying to take a screenshot of a span with zero height.

cy.get('.zero_height_element').screenshot();

Versions

Cypress 3.4.1
Ubuntu 16.04 & 18.04
@cypress-bot cypress-bot bot added the stage: ready for work The issue is reproducible and in scope label Sep 17, 2019
@jennifer-shehane
Copy link
Member

@egrubbs Thanks for the thorough debugging work. I can recreate this as you described.

<!DOCTYPE html>
<html>
<body>
    <div class='zero_height_element' style='height: 0'>abc</div>
</body>
</html>
it('screenshot zero height el', () => {
  cy.visit('index.html')
  cy.get('.zero_height_element').screenshot();
})

Screen Shot 2019-09-17 at 1 18 11 PM

We would love an open PR - even one that is a work in progress is fine. Check out our contributing doc and ask us if you get stuck.

@jennifer-shehane jennifer-shehane added topic: screenshots 📸 pkg/driver This is due to an issue in the packages/driver directory labels Sep 17, 2019
@sebinsua sebinsua mentioned this issue Sep 24, 2019
Merged
5 tasks
@jennifer-shehane jennifer-shehane mentioned this issue Sep 25, 2019
Open
@cypress-bot cypress-bot bot added stage: work in progress and removed stage: ready for work The issue is reproducible and in scope labels Sep 25, 2019
NMinhNguyen added a commit to sebinsua/cypress that referenced this issue Sep 26, 2019
@sebinsua @NMinhNguyen @jennifer-shehane
Handle 0px tall elements with a better error message
9b6a326 
Closes cypress-io#5149

Co-authored-by: Minh Nguyen <[email protected]>
Co-authored-by: Jennifer Shehane <[email protected]>
NMinhNguyen added a commit to sebinsua/cypress that referenced this issue Sep 27, 2019
@sebinsua @NMinhNguyen @jennifer-shehane
Handle 0px tall elements with a better error message
475f475 
Closes cypress-io#5149

Co-authored-by: Minh Nguyen <[email protected]>
Co-authored-by: Jennifer Shehane <[email protected]>
NMinhNguyen added a commit to sebinsua/cypress that referenced this issue Sep 27, 2019
@sebinsua @NMinhNguyen @jennifer-shehane
Handle 0px tall elements with a better error message
ea0863d 
Closes cypress-io#5149

Co-authored-by: Minh Nguyen <[email protected]>
Co-authored-by: Jennifer Shehane <[email protected]>
NMinhNguyen added a commit to sebinsua/cypress that referenced this issue Sep 30, 2019
@sebinsua @NMinhNguyen @jennifer-shehane
Handle 0px tall elements with a better error message
7ed56a5 
Closes cypress-io#5149

Co-authored-by: Minh Nguyen <[email protected]>
Co-authored-by: Jennifer Shehane <[email protected]>
@cypress-bot cypress-bot bot added stage: needs review The PR code is done & tested, needs review and removed stage: work in progress labels Oct 1, 2019
jennifer-shehane pushed a commit that referenced this issue Oct 11, 2019
@sebinsua @jennifer-shehane
Add padding support to element screenshot (#4440) (#5078)
116a634 
* Handle 0px tall elements with a better error message

Closes #5149

Co-authored-by: Minh Nguyen <[email protected]>
Co-authored-by: Jennifer Shehane <[email protected]>

* Add padding support to element#screenshot

Closes #4440

Co-authored-by: Minh Nguyen <[email protected]>
Co-authored-by: Jennifer Shehane <[email protected]>

* add e2e tests
@cypress-bot cypress-bot bot added stage: pending release and removed stage: needs review The PR code is done & tested, needs review labels Oct 11, 2019
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Oct 11, 2019

The code for this is done in cypress-io/cypress#5078, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Oct 23, 2019

Released in 3.5.0.

@snyk-bot snyk-bot mentioned this issue Nov 1, 2019
Merged
@snyk-bot snyk-bot mentioned this issue Nov 9, 2019
Closed
@Keysox Keysox mentioned this issue Dec 17, 2019
Open
@cypress-io cypress-io locked as resolved and limited conversation to collaborators Dec 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
pkg/driver This is due to an issue in the packages/driver directory topic: screenshots 📸 type: bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add padding support to element screenshot (#4440) sebinsua/cypress
2 participants
@egrubbs @jennifer-shehane