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

Throw error if cursor is returned from a method #5382

Closed
peonmodel opened this issue Oct 7, 2015 · 11 comments
Closed

Throw error if cursor is returned from a method #5382

peonmodel opened this issue Oct 7, 2015 · 11 comments
Labels

Comments

@peonmodel
Copy link

_1 Upvote_ repro here: https://github.com/peonmodel/adamant-waffle

basically is a server side meteor method is returning a cursor, the client side callback will not get called at all

    Meteor.methods({
            'test': function(obj){
                obj.fakefield += 1;
                delete obj._id;
                var id = TestCollection.insert(obj);
                var insert = TestCollection.find(id);  // <<< this line!
                return insert;
            }
        });
Meteor.call('test', previous, function(error, result){
                console.log('callback')  // will not get called
                if (error){
                    console.log(error)  // nor here
                } else {
                    console.log(result)  // nor here
                }
            })

it doesnt quite make sense to return a cursor from a method, but it is an easy mistake to make mistyping 'findOne' as 'find'
i would expect the error callback to get triggered instead of failing silently

@peonmodel
Copy link
Author

no, in the example i have posted, i didn't receive any error on the server,
i had just double checked, it fails silently on both client and server side

On Thu, Oct 8, 2015 at 11:02 AM, Sashko Stubailo notifications@github.com
wrote:

Is there any error on the server?


Reply to this email directly or view it on GitHub
#5382 (comment).

@jsbranco
Copy link

jsbranco commented Nov 1, 2015

+1 Same here.. findOne() returns triggers the callback but returning a cursor ( using find() ) there is no callback triggered.

The below are the scenarios I have tested:

//Doesn't trigger a callback
Meteor.methods({
   "retrievePageEvents": function (pageid) {
        return FacebookEvent.find({page: pageid});
    }
});

//Triggers callback
Meteor.methods({
   "retrievePageEvents": function (pageid) {
        return FacebookEvent.find({page: pageid}).fetch()
    }
});
//Triggers callback
Meteor.methods({
   "retrievePageEvents": function (pageid) {
        return FacebookEvent.findOne({page: pageid})
    }
});

EDIT : Also tested to just fire the "find" on the client side and it works fine.

//Returning cursor to template and handled correctly.
Template.activeFbEvents.helpers({
    pageEvents: function (routeData) {
        if (routeData) {

            return  FacebookEvent.find({page: routeData.pageid});;

        }

    }

I feel strongly that there might be an issue triggering the callback from a meteor method.

@peonmodel did you manage to resolve this issue?

@peonmodel
Copy link
Author

No, i did not, still awaiting a reply from the meteor team
I just take note to not return the cursor from a meteor method

@jsbranco
Copy link

jsbranco commented Nov 3, 2015

@peonmodel right.. I went for the same approach, if I need to return a cursor, I will do it in the helper itself.

Hopefully this gets fixed soon. I tried to find the source code for the method call but so far haven't found it

@stubailo
Copy link
Contributor

stubailo commented Nov 3, 2015

Yeah, returning a cursor from a method should throw an error, since cursors are not serializable.

@stubailo stubailo changed the title Bug? meteor method callback is not called if returning a cursor Throw error if cursor is returned from a method Nov 3, 2015
@zol zol removed the desired label Feb 23, 2016
@zol
Copy link
Contributor

zol commented May 31, 2016

Can someone confirm that returning anything that is non-serializable results in a silent error?

@zol
Copy link
Contributor

zol commented May 31, 2016

@peonmodel I can't access your repro

@peonmodel
Copy link
Author

peonmodel commented Jun 1, 2016

@zol i created a new repo: https://github.com/peonmodel/musical-journey

on client-side

Template.hello.events({
  'click button.obj'(event, instance) {
    Meteor.call('returnObject', (err, res)=>{
            console.log(err, res);  // gets called, get {name: 'Test'}
        });
  },
    'click button.cur'(event, instance) {
    Meteor.call('returnCursor', (err, res)=>{
            console.log(err, res);  // nothing <<<
        });
  },
    'click button.err'(event, instance) {
    Meteor.call('returnError', (err, res)=>{
            console.log(err, res);  // gets meteor error 'Test'
        });
  },
});

on server-side

Meteor.methods({
    'returnObject': function(){
        return TestCollection.findOne({name: 'Test'});
    },
    'returnCursor': function(){
        return TestCollection.find({name: 'Test'});
    },
    'returnError': function(){
        throw new Meteor.Error('error');
    },
});

@zol
Copy link
Contributor

zol commented Jun 1, 2016

Thanks!

@zol zol added confirmed We want to fix or implement it pull-requests-encouraged Impact:few labels Jun 1, 2016
@zbyte64
Copy link

zbyte64 commented Nov 18, 2016

Still an issue with meteor 1.4.2.1

@hwillson
Copy link
Contributor

hwillson commented Jun 9, 2017

To help provide a more clear separation between feature requests and bugs, and to help clean up the feature request backlog, Meteor feature requests are now being managed under the https://github.com/meteor/meteor-feature-requests repository.

This feature request will be closed here, but anyone interested in migrating this feature request to the new repository (to make sure it stays active), can click here to start the feature request migration process. This manual migration process is intended to help identify which of the older feature requests are still considered to be of value to the community. Thanks!

@hwillson hwillson closed this as completed Jun 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants