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

When downloading a file, error: "Cannot return null or undefined from an endpoint" #135

Open
andrewash opened this issue Sep 16, 2015 · 13 comments
Labels

Comments

@andrewash
Copy link

Thank you for putting this awesome library together!
I've created a custom "get" endpoint for one of my collections. This endpoint responds by writing an (MP3) file in the response and sending it to the client. Only authenticated responses are accepted (the security trimming is why I'm using Restivus for this, instead of regular HTTP download).

It works, but generates an error in Meteor's server console:

 Error: Cannot return null or undefined from an endpoint: get api/recordings/:id
     at packages/nimble:restivus/lib/route.coffee:61:25
     at packages/simple:json-routes/json-routes.js:45:1

I am calling this.done(), per the docs, so I'm not sure why the error occurs.

Anything I can do to eliminate the error message?

Thanks for your help,
Andrew

// Generates: GET on /api/recordings
API.addCollection(Recordings, {
  excludedEndpoints: ['post', 'put', 'delete'],
  routeOptions: {
    authRequired: true,
    roleRequired: ['student', 'teacher']
  },
  endpoints: {
    get: {
      action: function() {
        // GET api/recordings/:id
        // tested with: wget --content-disposition http://localhost:3000/api/recordings/NaxN2DLQThpaH4k7Z
        // this saves the file with the correct filename
        if (!this.urlParams.id) {
         return {
           statusCode: 400,
           body: {
             status: "failure",
             message: "Missing Recording ID"
           }
         };
        }                
        // Construct the query
        var permissionsFilter = API.buildQueryFilterForPermittedRecordings(this.userId);
        var recordingId = this.urlParams.id;
        var query = {$and: [permissionsFilter, {_id: recordingId}]};
        var recording = Recordings.findOne(query);
        if (!recording) {
         return {
           statusCode: 400,
           body: {
             status: "failure",
             message: "Invalid Recording ID"
           }
         };
        }

        var destFilename = Recordings.buildFilename(recording);
        var filePath = process.env.PWD + recording.url;
        var data = fs.readFileSync(filePath);            
        this.response.writeHead(200, {
          'Content-disposition': "attachment; filename=" + destFilename,
          'Content-type': 'audio/mpeg3',
          'Content-length': data.length
        });        
        this.response.write(data);
        this.done();
      }
    }
  }
});
@kahmali
Copy link
Owner

kahmali commented Sep 20, 2015

Everything looks great there at first glance. This is a tested feature, so, if you're experiencing a potential bug, I'll either need reproduction steps or (even better) a sample repo demonstrating the issue. That should help me get to the bottom of it. Sorry that you're experiencing the issue. I hope we can get this resolved for you asap. Also, thanks for the compliment, and thanks for reporting this!

@mehar
Copy link

mehar commented Nov 16, 2015

Returning an empty object saved me from the same exception!!

@kahmali
Copy link
Owner

kahmali commented Nov 17, 2015

@mehar Thanks for the suggestion. The problem here is that it appears @andrewash is always returning something from that endpoint, except when they manually handled the response, in which case calling this.done() should suffice. So without a sample repo or detailed repro steps to test this against, it's difficult to tell. If your suggested fix works, that's awesome, but I'd like to get to the bottom of it because that's not the intended behavior. Calling this.done() should skip the check for an empty endpoint response, and allow you to proceed without error.

@mehar
Copy link

mehar commented Nov 17, 2015

@kahmali Reproduction steps are quite simple

  1. Define an endpoint
  2. Handle the response by yourself by not returning anything but by do end with this.done()

You will see the above mentioned exception

Add a return {}; statement at the end and see everything works as expected

@kahmali
Copy link
Owner

kahmali commented Nov 17, 2015

Alright. I'll test that out asap. As you can see there is an automated test in place for this that is passing in local and CI tests: https://github.com/kahmali/meteor-restivus/blob/devel/test/api_tests.coffee#L229. Not sure how that's possible, but I'll look into as soon as I have a chance. Thanks for providing the repro steps @mehar!

@kahmali
Copy link
Owner

kahmali commented Nov 17, 2015

Oh, I see. It works, but it still throws the error on the console. Ahhh. That should be an easy fix. I'll look into that. Sorry for the misunderstanding.

@kahmali kahmali added the bug label Nov 17, 2015
@mehar
Copy link

mehar commented Nov 18, 2015

Thanks @kahmali , btw thanks for the awesome library!!!

@isdzulqor
Copy link

How to do this in restivus,, I wanna make a server for handling a file upload from my android native client. Below is the code that's implemented in node js that i get from tutorial in internet. How to do like this below in meteor js especially with this library restivus
Thank you

var fs = require('fs');

module.exports = function(app) {

app.get('/',function(req,res){
res.end("Node-File-Upload");

});
app.post('/upload', function(req, res) {
console.log(req.files.image.originalFilename);
console.log(req.files.image.path);
fs.readFile(req.files.image.path, function (err, data){
var dirname = "/home/rajamalw/Node/file-upload";
var newPath = dirname + "/uploads/" + req.files.image.originalFilename;
fs.writeFile(newPath, data, function (err) {
if(err){
res.json({'response':"Error"});
}else {
res.json({'response':"Saved"});
}
});
});
});

app.get('/uploads/:file', function (req, res){
file = req.params.file;
var dirname = "/home/rajamalw/Node/file-upload";
var img = fs.readFileSync(dirname + "/uploads/" + file);
res.writeHead(200, {'Content-Type': 'image/jpg' });
res.end(img, 'binary');

});
};

@a4xrbj1
Copy link

a4xrbj1 commented Jan 19, 2016

Same problem here (Error: Cannot return null or undefined from an endpoint), it's just that I'm trying to return statusCode 401. I've tried it like this:

if (err) {
                return {
                    statusCode: 401
                };
            } else {
                return {
                    statusCode: 200,
                    body: {status: 'success', message: 'Ok'}
                };
            }

The statusCode 200 goes through without problems at all.

Detailed error message is:

Error: Cannot return null or undefined from an endpoint: post api/login/2
at Route.Route.addToApi._.each.JsonRoutes.add.responseData.status (packages/nimble:restivus/lib/route.coffee:61:25)
at JsonRoutes.add (packages/simple:json-routes/json-routes.js:45:1)

Not sure if it's the same bug causing it. I've also tried it with returning a statusCode and an Object (similar to statusCode 200 object) but to no avail.

@kahmali
Copy link
Owner

kahmali commented Jan 19, 2016

Hi @a4xrbj1,

I probably did it trying to enforce "best practices," but you're required to return a body alongside any statusCode or headers. If anyone would like to submit a PR with the requirement removed, I'd gladly accept that. In the meantime, just add an empty body (string or object) in your error response. So your code should look like:

if (err) {
                return {
                    statusCode: 401,
                    body: {}
                };
            } else {
                return {
                    statusCode: 200,
                    body: {status: 'success', message: 'Ok'}
                };
            }

I believe it's best practice to return an error message in the body in error responses, but to each their own. I'd personally suggest returning something like this (to follow the JSend response standard we use throughout Restivus, for consistent response format):

if (err) {
                return {
                    statusCode: 401,
                    body: {status: 'error', message: 'Unauthorized'}
                };
            } else {
                return {
                    statusCode: 200,
                    body: {status: 'success', message: 'Ok'}
                };
            }

Hope that helps!

@LaughingBubba
Copy link

Hi there, I get the same issue when uploading. I'm trying to use the multer npm package with restivus to add a file upload function to some restful api.

The irony is that although restivus throws an error the file does get uploaded!

I get this:
'(STDERR) Error: Cannot return null or undefined from an endpoint: post api/uploads/
(STDERR) at packages/nimble_restivus/lib/route.coffee:61:25
(STDERR) at packages/simple_json-routes/json-routes.js:76:1

If I add a return {} then the there's no error but then multer hangs. I've made a quick and dirty repo to demonstrate: https://github.com/LaughingBubba/uploads-meteor

Thoughts?

Cheers

@kahmali
Copy link
Owner

kahmali commented Jun 16, 2016

Hi @LaughingBubba,

Sorry to you and everyone else for the delay on fixing this. I completely overlooked it. I just published v0.8.11 to address the issue outlined by @mehar above. Hopefully that fixes the hanging in multer for you. I didn't have time to write a new test, so I'm going to keep this issue open until you verify that it works for you. Thanks for reporting your issue, and sorry again to you and anyone else that's been waiting on this fix!

@kahmali kahmali reopened this Jun 16, 2016
@kahmali kahmali removed the question label Jun 16, 2016
@LaughingBubba
Copy link

No worries! Thanks for the update. I'll check now and let you know.

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