Raises on INSUFFICIENT_PERMISSION error from NetSuite response

[aom]

We're handling a NetSuite installation with multiple Subsidiaries and during development we're seeing a lot of INSUFFICIENT_PERMISSION errors from Subsidiary restrictions. Especially when searching for subsidiaries from NetSuite.

In development these are easy to spot from XML responses but in production they would get lost without verbose logging. We consider these the kind of errors to raise because they won't heal themselves automatically.

Example SOAP request and response:

<env:Envelope xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:platformMsgs="urn:messages_2017_2.platform.webservices.netsuite.com" xmlns:env="http://schemas.xmlsoap.org/soap/envelope/" xmlns:platformCore="urn:core_2017_2.platform.webservices.netsuite.com" xmlns:platformCommon="urn:common_2017_2.platform.webservices.netsuite.com" xmlns:listRel="urn:relationships_2017_2.lists.webservices.netsuite.com" xmlns:tranSales="urn:sales_2017_2.transactions.webservices.netsuite.com" xmlns:tranPurch="urn:purchases_2017_2.transactions.webservices.netsuite.com" xmlns:actSched="urn:scheduling_2017_2.activities.webservices.netsuite.com" xmlns:setupCustom="urn:customization_2017_2.setup.webservices.netsuite.com" xmlns:listAcct="urn:accounting_2017_2.lists.webservices.netsuite.com" xmlns:tranBank="urn:bank_2017_2.transactions.webservices.netsuite.com" xmlns:tranCust="urn:customers_2017_2.transactions.webservices.netsuite.com" xmlns:tranEmp="urn:employees_2017_2.transactions.webservices.netsuite.com" xmlns:tranInvt="urn:inventory_2017_2.transactions.webservices.netsuite.com" xmlns:listSupport="urn:support_2017_2.lists.webservices.netsuite.com" xmlns:tranGeneral="urn:general_2017_2.transactions.webservices.netsuite.com" xmlns:commGeneral="urn:communication_2017_2.general.webservices.netsuite.com" xmlns:listMkt="urn:marketing_2017_2.lists.webservices.netsuite.com" xmlns:listWebsite="urn:website_2017_2.lists.webservices.netsuite.com" xmlns:fileCabinet="urn:filecabinet_2017_2.documents.webservices.netsuite.com" xmlns:listEmp="urn:employees_2017_2.lists.webservices.netsuite.com">
  <env:Header>
    ..
  </env:Header>
  <env:Body>
    <platformMsgs:search>
      <platformMsgs:searchRecord xsi:type="listAcct:SubsidiarySearch">
        <listAcct:basic/>
      </platformMsgs:searchRecord>
    </platformMsgs:search>
  </env:Body>
</env:Envelope>

Response

<soapenv:Envelope xmlns:soapenv="http://schemas.xmlsoap.org/soap/envelope/" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
  <soapenv:Header>
    <platformMsgs:documentInfo xmlns:platformMsgs="urn:messages_2017_2.platform.webservices.netsuite.com">
      <platformMsgs:nsId>..</platformMsgs:nsId>
    </platformMsgs:documentInfo>
  </soapenv:Header>
  <soapenv:Body>
    <searchResponse xmlns="urn:messages_2017_2.platform.webservices.netsuite.com">
      <platformCore:searchResult xmlns:platformCore="urn:core_2017_2.platform.webservices.netsuite.com">
        <platformCore:status isSuccess="false">
          <platformCore:statusDetail type="ERROR">
            <platformCore:code>INSUFFICIENT_PERMISSION</platformCore:code>
            <platformCore:message>Permission Violation: The subsidiary restrictions on your role prevent you from seeing this record.</platformCore:message>
          </platformCore:statusDetail>
        </platformCore:status>
      </platformCore:searchResult>
    </searchResponse>
  </soapenv:Body>
</soapenv:Envelope>

[freezepl]

Hey @iloveitaly , could you comment or merge this PR? I have similar issue and I think more people are affected by this..

[iloveitaly]

@freezepl @aom I like this idea! Going to need some more time to think on it and see if it any unintended side effects. Short on time right now!

[freezepl]

@iloveitaly thanks! we could add feature flag, so it would not break old implementations.

[JakubKopys]

@iloveitaly any updates? Would love to see this merged!

[iloveitaly]

@JakubKopys not yet :( Pretty busy with the day-job at the moment.

[aom]

Feature flag is probably a good idea as this might cause unexpected exceptions for people using this gem. I'm happy to implement the flag!

One option is to have simple boolean config to NetSuite.configure block. Another would be to allow developer to configure list of errors to raise. Ie.

NetSuite.configure do
  raise_on_response_errors ['INSUFFICIENT_PERMISSION']
end

[freezepl]

Feature flag is probably a good idea as this might cause unexpected exceptions for people using this gem. I'm happy to implement the flag!

One option is to have simple boolean config to NetSuite.configure block. Another would be to allow developer to configure list of errors to raise. Ie.

NetSuite.configure do
  raise_on_response_errors ['INSUFFICIENT_PERMISSION']
end

I think this is right approach. Although I would use only

NetSuite.configure do
  raise_on_response_errors true
end

[aom]

@freezepl Agreed. As long as we're only handling one response error it should not be any more complicated to turn the feature on. I did the gate now.

I think I'll need to update this to README.md next if this is what we'll want to merge?

[freezepl]

Thanks @aom this looks awesome!

Ideally this option should be a default one, but it is a breaking change. It could be released under v 1.X version in the future.

@iloveitaly what do you think about this PR now? I would give 🚢

[iloveitaly]

Why should we gate this behind a feature flag? Given that this is less than < 1.0 I don't see any reason why we can't introduce breaking changes, especially if we document them in the release notes.

I think raising an exception on these sort of hard NetSuite failures feels like the correct default behavior and I can't imagine a world where folks wouldn't want an exception to be raised.

Let me know what I'm missing here!

[freezepl]

@iloveitaly you commented here about unintended side effects, so we thought it can be turn on in the configuration..

Maybe we can do it other way round ? Turn it off in configuration?

[aom]

How about this:

With raise_on_response_errors being false by default it would not be a breaking change and could already be merged to master.

Then at 1.0 you could change the default to true and add it as a breaking change to release notes? There could be a simple PR labeled with 1.0 for this change.

[aom]

I agree with @iloveitaly that these sort of hard NetSuite failures should raise instead fail silently. It would've been especially helpful when we we're setting up the access management for our project :)

But including it in README.md's configuration example might do the trick without breaking changes.

[freezepl]

@iloveitaly any updates on this PR? Can we merge it?

[freezepl]

Bump! Guys would like to use this feature and I don't want to use fork. Can we merge it? @iloveitaly

[freezepl]

@iloveitaly any thoughts ?

[freezepl]

@iloveitaly can we merge this PR ?

[iloveitaly]

Sorry, haven't had time to look at this! I need to run some tests on other systems first. Not a priority at the moment, unfortunately :(

[freezepl]

5 moths later @iloveitaly could we do something about this PR ? (after fixing conflicts)

[aom]

@freezepl @iloveitaly Rebased with master to resolve conflicts.

[aom]

I'm no longer maintaining the fork. Best people to have a chat about changes to this PR are @beyondcompute and @edimossilva