aws-amplify / amplify-category-api

The AWS Amplify CLI is a toolchain for simplifying serverless web and mobile development. This plugin provides functionality for the API category, allowing for the creation and management of GraphQL and REST based backends for your amplify project.
https://docs.amplify.aws/
Apache License 2.0
89 stars 77 forks source link

Bug in code #2785

Closed SanjoSolutions closed 1 month ago

SanjoSolutions commented 2 months ago

How did you install the Amplify CLI?

No response

If applicable, what version of Node.js are you using?

No response

Amplify CLI Version

12.12.4

What operating system are you using?

Windows 11

Did you make any manual changes to the cloud resources managed by Amplify? Please describe the changes made.

No.

Describe the bug

In @aws-amplify/data-schema/dist/esm/runtime/internals/operations/custom.mjs:312 [null] can be passed to initializeModel, which initializeModel does not seem to handle correctly (it throws an error in my case). In my case isArrayResult === false and flattenedResult === null. It seems that this bug can be fixed by first converting the single value to an array and then doing the .filter((x) => x) from line 307, so that [] is passed to initializeModel instead of [null].

The error that was thrown in my case is:

APIClient.mjs:267 Uncaught (in promise) TypeError: Cannot read properties of null (reading 'id')
    at eval (APIClient.mjs:267:49)
    at Array.map (<anonymous>)
    at eval (APIClient.mjs:265:1)
    at Array.map (<anonymous>)
    at initializeModel (APIClient.mjs:140:1)
    at _op (custom.mjs:312:34)
    at async f (useOrganization.ts:24:24)

Expected behavior

No error.

Reproduction steps

Please see "Describe the bug" for the conditions under which the error seems to occur.

Project Identifier

No response

Log output

``` # Put your logs below this line ```

Additional information

No response

Before submitting, please confirm:

svidgen commented 2 months ago

Thanks for the suggestion! But, do you have repro steps for this? I'm not sure offhand what scenario leads to that null-value-array there. And, if this of still a current bug, I'll want to write a regression test for the high level behavior alongside the fix.

Thanks!

SanjoSolutions commented 2 months ago

In my case a Lambda GraphQL query handler (retrieveOrganization) has returned null. The return type of this handler is a.ref('Organization'). The organization model has a hasMany relationship (members). Because of this relationship and because null has been returned it seems that the error, that I have posted in my initial post, has occurred. I provide the code in this post under the MIT-0 license.

On the client side (in a Next.js app):

const result = await client.queries.retrieveOrganization()

On the server side:

amplify/data/resource.ts:

import { type ClientSchema, a, defineData } from '@aws-amplify/backend'
import { retrieveOrganization } from './retrieve-organization/resource.js'

const schema = a
  .schema({
    Organization: a
      .model({
        id: a.id().required(),
        owner: a.string().required(),
        name: a.string(),
        members: a.hasMany('OrganizationMember', 'organizationID'),
        areApplicationsEnabled: a.boolean().default(false),
      })
      .authorization(allow => [allow.owner().to([])]),
    OrganizationMember: a
      .model({
        owner: a.string().required(),
        organizationID: a.id(),
        organization: a.belongsTo('Organization', 'organizationID'),
      })
      .secondaryIndexes(index => [index('owner')])
      .authorization(allow => [allow.owner().to([])]),
    retrieveOrganization: a
      .query()
      .returns(a.ref('Organization'))
      .handler(a.handler.function(retrieveOrganization))
      .authorization(allow => [allow.authenticated()]),
   })
  .authorization(allow => [
    allow.resource(retrieveOrganization)
  ])

export type Schema = ClientSchema<typeof schema>

export const data = defineData({
  schema,
  authorizationModes: {
    defaultAuthorizationMode: 'userPool',
    apiKeyAuthorizationMode: {
      expiresInDays: 30,
    },
  },
})

amplify/retrieve-organization/resource.ts:

import { defineFunction } from '@aws-amplify/backend'

export const retrieveOrganization = defineFunction({
  runtime: 20,
})

amplify/retrieve-organization/handler.ts:

import type { Schema } from '../resource'
import { Amplify } from 'aws-amplify'
import { env } from '$amplify/env/retrieve-organization-member'

Amplify.configure(
  {
    API: {
      GraphQL: {
        endpoint: env.AMPLIFY_DATA_GRAPHQL_ENDPOINT,
        region: env.AWS_REGION,
        defaultAuthMode: 'iam',
      },
    },
  },
  {
    Auth: {
      credentialsProvider: {
        getCredentialsAndIdentityId: async () => ({
          credentials: {
            accessKeyId: env.AWS_ACCESS_KEY_ID,
            secretAccessKey: env.AWS_SECRET_ACCESS_KEY,
            sessionToken: env.AWS_SESSION_TOKEN,
          },
        }),
        clearCredentialsAndIdentityId: () => {
          /* noop */
        },
      },
    },
  }
)

export const handler: Schema['retrieveOrganization']['functionHandler'] =
  async event => {
    return null
  }
svidgen commented 2 months ago

@SanjoSolutions On the surface, this looks like a bug, but my initial attempts to repro have been unsuccessful here. Can you tell me what version of data-schema you have installed?

SanjoSolutions commented 2 months ago

@SanjoSolutions On the surface, this looks like a bug, but my initial attempts to repro have been unsuccessful here. Can you tell me what version of data-schema you have installed?

1.3.10

SanjoSolutions commented 2 months ago

I have noticed that there is a catch block at @aws-amplify/data-schema/dist/esm/runtime/internals/operations/custom.mjs line 325-337 which the code runs through in my case. In line 333 it seems to try to retrieve data from the error. In my case the error does not have a value for data. So the logic that follows that line seems to fail.

In line 299 data is {"retrieveOrganization": null} in my case.

SanjoSolutions commented 2 months ago

@svidgen I have made a repo with a small app with which the error can be reproduced: https://github.com/SanjoSolutions/bug-repro

stocaaro commented 1 month ago

Hello @SanjoSolutions ,

Thank you for putting together this sample app. I have used it to confirm the behavior you described above. I think I see where the error is coming from in the library and will be taking a deeper look tomorrow.

stocaaro commented 1 month ago

I put a fix out earlier today and have tested it with the bug-repo to verify that it works. Thanks again for the info. Updating to the latest version of @aws-amplify/data-schema >=1.6.1 will pick up the fix.

I'm resolving the issue, but will watch for any further updates.

github-actions[bot] commented 1 month ago

This issue is now closed. Comments on closed issues are hard for our team to see. If you need more assistance, please open a new issue that references this one.