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

return-netsuite-error-for-search-action-failures #485

Conversation

Timothyjb
Copy link
Contributor

Why
Currently, if the response of the Search action is not successful, it will simply return false and give no insight into what the issue was.
Looking at patterns implemented elsewhere in the codebase, we seem to be moving towards returning a NetSuite::Error if the response is not successful.

Solution
Implement returning a NetSuite::Error if the response is not successful.

Why
Currently, if the `Search` action is unsuccessful, it will simply return `false` and give no insight into what the error was.
Looking at patterns implemented elsewhere in the codebase, we seem to be moving towards returning a `NetSuite::Error` if the response is not successful.

Solution
Return a `NetSuite::Error` if the response is not successful.
@iloveitaly
Copy link
Member

@Timothyjb where else in the codebase are you seeing this pattern?

I think this, combined with something like #405, is the right approach going forward. Just want to think on/validate the approach a bit more when I have some time.

@aom
Copy link

aom commented Sep 27, 2021

@iloveitaly Would be awesome to see some progress on this after such a long time. We've been running a fork of this repository to get this feature to our production.

@iloveitaly
Copy link
Member

@aom Me too! Just time constrained on my end :/

@Timothyjb
Copy link
Contributor Author

Going to close this one for now. Depending on how #405 turns out, this may not be necessary. I can probably revisit then.

@Timothyjb Timothyjb closed this Oct 28, 2021
@gbs4ever
Copy link
Contributor

it would make sense that if doing a search that is rejected to return Permission Violation: You need the and not just false.

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.

4 participants