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

Include stack in Cloud Code #6958

Merged
merged 2 commits into from
Oct 21, 2020
Merged

Conversation

dblythy
Copy link
Member

@dblythy dblythy commented Oct 21, 2020

At the moment in Cloud Code, if an error is thrown from a syntax / JS error, such as:

Parse.Cloud.define("testFunction", async (request) => {
    yolo
});

The following error is shown:

error: Parse error:  yolo is not defined {"code":141,"stack":"Error: yolo is not defined
    at error (/node_modules/parse-server/lib/Routers/FunctionsRouter.js:121:16)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)}"

Which can make it a bit difficult sometimes to trace when your main.js is long.

This PR passes stack through if the error thrown is an Error, meaning that the following is logged, making it a bit easier to identify where the syntax error was.

error: Parse error:  yolo is not defined {"code":141,"stack":"ReferenceError: yolo is not defined
    at  /cloud/main.js:3:5
    at /node_modules/parse-server/lib/Routers/FunctionsRouter.js:199:16
    at processTicksAndRejections (internal/process/task_queues.js:93:5)"}

Sorry for submitting so many PRs!

@ghost
Copy link

ghost commented Oct 21, 2020

Danger run resulted in 1 fail and 1 markdown; to find out more, see the checks page.

Generated by 🚫 dangerJS

@dplewis
Copy link
Member

dplewis commented Oct 21, 2020

I was just thinking about this error handling.

@codecov
Copy link

codecov bot commented Oct 21, 2020

Codecov Report

Merging #6958 into master will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6958      +/-   ##
==========================================
- Coverage   93.80%   93.79%   -0.02%     
==========================================
  Files         169      169              
  Lines       12254    12255       +1     
==========================================
- Hits        11495    11494       -1     
- Misses        759      761       +2     
Impacted Files Coverage Δ
src/Routers/FunctionsRouter.js 94.93% <100.00%> (+0.06%) ⬆️
src/Adapters/Storage/Mongo/MongoStorageAdapter.js 92.92% <0.00%> (-0.67%) ⬇️
src/RestWrite.js 93.82% <0.00%> (+0.16%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 72428dc...97d46f5. Read the comment docs.

@dblythy
Copy link
Member Author

dblythy commented Oct 21, 2020

@dplewis would you have any suggestions / pointers as to how to get relevant trace if a Parse.Error is thrown? The solution in this PR only works if if a JS error is thrown.

E.g:

Parse.Cloud.define("testFunction", async (request) => {
    let obj = new Parse.Object('Test');
    await obj.save();
    await obj.destroy();
    await obj.save();
});

Throws

error: Parse error:  Object not found. {"code":101,"stack":"Error: Object not found.
    at /node_modules/parse-server/lib/Controllers/DatabaseController.js:563:23
    at processTicksAndRejections (internal/process/task_queues.js:93:5)"}

Which again, isn't overly helpful at identifying where the error is comes from. Obviously, you can wrap it in a try / catch block, but just thinking how we can make it as novice-friendly as possible.

@dplewis
Copy link
Member

dplewis commented Oct 21, 2020

There was a conversation about this. #6205 (comment)

One place to start would be to remove the promise chaining that happens in the rest, restquery, restwrite, etc. I can do this in a separate PR. There may be simpler solution.

Edit: its easier to test this with directAccess: true

Copy link
Member

@davimacedo davimacedo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@davimacedo davimacedo merged commit c647c53 into parse-community:master Oct 21, 2020
@dblythy dblythy deleted the CloudError branch October 22, 2020 05:10
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

Successfully merging this pull request may close these issues.

3 participants