Skip to content

Exception when restoring methods under phantomjs #306

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

Closed
giulianob opened this issue Jul 18, 2013 · 10 comments
Closed

Exception when restoring methods under phantomjs #306

giulianob opened this issue Jul 18, 2013 · 10 comments

Comments

@giulianob
Copy link

When running our tests in phantomjs, we're seeing an exception that gets thrown when sinonjs is restoring sessionStorage.getItem method. It happens at the following "delete" call:

            method.restore = function () {
                // For prototype properties try to reset by delete first.
                // If this fails (ex: localStorage on mobile safari) then force a reset
                // via direct assignment.
                if (!owned) {
                    delete object[property];
                }
                if (object[property] === method) {
                    object[property] = wrappedMethod;
                }
            };

After some research, it appears that delete will throw exceptions when javascript is running in strict mode if it's unable to delete a property. From Mozilla's page: "Third, strict mode makes attempts to delete undeletable properties throw (where before the attempt would simply have no effect):" In Chrome, when we delete sessionStorage.getItem after it's wrapped it simply sets it back to the native implementation. In Phantom, it throws an exception if it's unable to delete. It looks like the comment above already expects cases where the delete fails and the method needs to be restored manually but it doesnt expect cases where the delete throws an exception.

Would it be possible to add a try/catch around the delete for strict browsers?

@mantoni
Copy link
Member

mantoni commented Jul 18, 2013

To add some context: Sinon has two ways of stubbing and the corresponding reverse operation:

  • Replace a function on an object: The original function is stored and assigned back on restore.
  • Add a function to an object to override a prototype implementation: The override is deleted on restore.

In your case, Sinon is apparently using the second approach to stub the getItem function. That means, it's not attempting to delete a not deletable function, it's trying to delete the stub function that it put there itself.

Just putting a try-catch around it will leave the stub in place which is not really solving your problem.

I'm wondering whether this is produceable in other (WebKit) browsers?

@giulianob
Copy link
Author

It doesn't seem to leave the stub in place since the lines below will check
if it's still stubbed and replace it. I am however running into another
issue which is sinon being unable to stub out sessionStorage.getItem in
Firefox.

On Thu, Jul 18, 2013 at 1:10 PM, Maximilian Antoni <[email protected]

wrote:

To add some context: Sinon has two ways of stubbing and the corresponding
reverse operation:

  • Replace a function on an object: The original function is stored and
    assigned back on restore.
  • Add a function to an object to override a prototype implementation:
    The override is deleted on restore.

In your case, Sinon is apparently using the second approach to stub the
getItem function. That means, it's not attempting to delete a not
deletable function, it's trying to delete the stub function that it put
there itself.

Just putting a try-catch around it will leave the stub in place which is
not really solving your problem.

I'm wondering whether this is produceable in other (WebKit) browsers?


Reply to this email directly or view it on GitHubhttps://github.com//issues/306#issuecomment-21198875
.

Giuliano Barberi

@scinos
Copy link

scinos commented Aug 6, 2013

Found similar problem when stubbing localStorage.getItem.

//Sinon's wrapMethod
function wrapMethod(object, property, method) {
    // Some checks here

    // At this point, the following asserts are true:
    //     object == localStorage
    //     property == "getItem"
    //     typeof method == "function"
    //     typeof object[property] == "function"
    // So far so good
    object[property] = method; 

    //But after the reassignment:
    //     object == localStorage
    //     property == "getItem"
    //     typeof method == "function"
    //     typeof object[property] == "string"  <-- bug?
}

@giulianob Did you find any workaround or more info about this behaviour?

@scinos
Copy link

scinos commented Aug 6, 2013

Oh, found the problem.

You can use standard JS access to get/set the properties, like:

localStorage.setItem("myKey", "123");
localStorage.getItem("myKey") //"123"
localStorage.myKey2 = "123"
localStorage.myKey2 //"123"

So in FF, you can actually store a property called "getItem"

typeof localStorage.getItem == "function" //true
localStorage.getItem = "xx" //equivalent to 'localStorage.setItem('getItem', 'xx')
typeof localStoraget.getItem == "string" //true
localStoraget.getItem("something") //TypeError: localStorage.getItem is not a function

@fredericfalliere
Copy link

bump on this !

I cannot restore() spy/stub/mock in phantomjs.
Works in the browser btw.

What do ?

@cjohansen
Copy link
Contributor

I'm not sure what to do about this. Looking at @scinos comment, it seems this may not be possible to solve in a satiesfactory way? Would it work to Object.defineProperty to create the stub? That would require some feature forking, but might be a solution.

@cjohansen
Copy link
Contributor

Not sure what to do here. I would suggest not stubbing directly on native objects. Please re-open if you feel there is something Sinon can do to help you here.

@nmabhinandan
Copy link

Cannot restore sinon spies in phantom js. But it's working fine in chrome

@dynaum
Copy link

dynaum commented May 19, 2015

+1

@GCheung55
Copy link
Contributor

Could you describe your setup or share code to reproduce the issue?

On May 19, 2015, at 8:05 AM, Elber Ribeiro [email protected] wrote:

+1


Reply to this email directly or view it on GitHub.

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

No branches or pull requests

8 participants