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

Fix handling of anonymous object types #472

Merged
merged 1 commit into from Oct 7, 2019

Conversation

@bajtos
Copy link
Member

bajtos commented Oct 4, 2019

In LoopBack, the method PersistedModel.updateAll describes the return type using an inline (anonymous) definition:

{
  returns: {
    arg: 'info',
    type: {
      count: {
        type: 'number',
        description: 'The number of instances updated',
      },
    },
    root: true,
  },
}

Before this change, remote invocation of updateAll was crashing the process with the following error:

AssertionError: type must be either an array or a string.

This patch fixes the problem as follows:

  • A try/catch block is introduced to correctly report any sync errors via the callback

  • The type registry was improved to recognize anonymous object types and use the "object" converter for them.

Related issues

Fixes strongloop/loopback#3717

/cc @angfal @ewrayjohnson

Checklist

  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
Copy link
Member

hacksparrow left a comment

I don't see any obvious issues. LGTM.

test/rest.browser.test.js Outdated Show resolved Hide resolved
Copy link
Contributor

jannyHou left a comment

LGTM 👍 just commented a question

In LoopBack, the method `PersistedModel.updateAll` describes the return
type using an inline (anonymous) definition:

  returns: {
    arg: 'info',
    type: {
      count: {
        type: 'number',
        description: 'The number of instances updated',
      },
    },
    root: true,
  },

Before this change, remote invocation of updateAll was crashing the
process with the following error:

  AssertionError: type must be either an array or a string.

This commit fixes the problem as follows:

- A try/catch block is introduced to correctly report any sync errors
  via the callback

- The type registry was improved to recognize anonymous object types
  and use the "object" converter for them.

Signed-off-by: Miroslav Bajtoš <mbajtoss@gmail.com>
@bajtos bajtos force-pushed the fix/loopback-remote-invoke-updateAll branch from d2872ac to d60b69b Oct 7, 2019
@bajtos bajtos merged commit 1cb6f10 into master Oct 7, 2019
16 checks passed
16 checks passed
Commit Linter commits are all properly formatted
Details
PR Linter PR is up to date
Details
[cis-jenkins] PR Builder Build finished.
Details
[cis-jenkins] downstream: loopback-connector-remote@master Success! (d60b69b)
Details
[cis-jenkins] downstream: loopback@master Success! (d60b69b)
Details
[cis-jenkins] x64 && linux && nvm,10 Success! (d60b69b)
Details
[cis-jenkins] x64 && linux && nvm,12 Success! (d60b69b)
Details
[cis-jenkins] x64 && linux && nvm,8 Success! (d60b69b)
Details
clahub All contributors have signed the Contributor License Agreement.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.04%) to 87.763%
Details
pr-builder
Details
security/snyk - package.json (StrongLoop) No manifest changes detected
strong-remoting Success! (d60b69b)
Details
strong-remoting/node=4.x,os=windows Success! (d60b69b)
Details
strong-remoting/node=6.x,os=windows Success! (d60b69b)
Details
@delete-merged-branch delete-merged-branch bot deleted the fix/loopback-remote-invoke-updateAll branch Oct 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.