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 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 hacksparrow left a comment

Choose a reason for hiding this comment

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

I don't see any obvious issues. LGTM.

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

@jannyHou jannyHou left a comment

Choose a reason for hiding this comment

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

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š <[email protected]>
@bajtos bajtos force-pushed the fix/loopback-remote-invoke-updateAll branch from d2872ac to d60b69b Compare October 7, 2019 08:37
@bajtos bajtos merged commit 1cb6f10 into master Oct 7, 2019
@delete-merged-branch delete-merged-branch bot deleted the fix/loopback-remote-invoke-updateAll branch October 7, 2019 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PersistedModel's updateAll method crashes the server when invoked via the remote connector
3 participants