neutralinojs / neutralino.js

JavaScript API for Neutralinojs
https://neutralino.js.org/docs/api/overview
MIT License
237 stars 47 forks source link

Enhanced Error Reporting for Improved Debugging and User Experience #100

Closed Sadaf-A closed 7 months ago

Sadaf-A commented 7 months ago

Resolves: #99

This PR adds a new class for throwing errors and gives more descriptive errors

This is what our errors look like now: error(1)

This is what they will look like after the changes introduced by this PR are merged: error-now

This PR also makes the sendMessage() method in websocket.ts async which is required to get the proper error object

Sadaf-A commented 7 months ago

just made a commit which sends the error code along with the message now the error message looks like this: code

Sadaf-A commented 7 months ago

remove unnecessary spaces in websocket.ts

hey @Kanai2003, thanks for your review those spaces are for indentation. Any issue related to formatting will be resolved if my PR for integrating es-lint and prettier gets merged

Kanai2003 commented 7 months ago

Oh! my bad!

Sadaf-A commented 7 months ago

Hello @Sadaf-A, Thanks for sending this pull request. I would like to skip adding these suggestions due to the following reasons:

* It's up to app developers handle Neutralinojs errors - not the framework itself. i.e., `clipboard.writeText()` throws an error, so app developers need to handle it with `try/catch`

* We realdy expose error code types for TypeScript

However, if more app developers mention that they need to see all uncaught errors on the console as formatted error messages, we can do something like this I think:

nativeCalls[message.id].reject(message.data.error);
if(onDebugMode) {
     logError(); // <----
}

Let's skip adding this feature for now 🎉

Hey, thanks for the review. From that perspective it makes more sense to keep things the way they are!