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

table.query is not a function #236

Closed
volkanunsal opened this issue Jun 12, 2022 · 15 comments
Closed

table.query is not a function #236

volkanunsal opened this issue Jun 12, 2022 · 15 comments

Comments

@volkanunsal
Copy link
Contributor

I tried using Table inside a Function, and got this error. The generated code seems to be missing every method. Did I do this right?

ddbTable.addGlobalSecondaryIndex({
  indexName: 'GSI1',
  partitionKey: { name: 'PK', type: aws_dynamodb.AttributeType.STRING },
  sortKey: { name: 'SK', type: aws_dynamodb.AttributeType.STRING },
  projectionType: aws_dynamodb.ProjectionType.ALL,
});

const table = new Table<ITable, 'PK', any>(ddbTable);

const defaultAuthFunction = new Function<AuthContext, AuthResponse>(
  this,
  'LambdaAuthorizer',
  {
    logRetention: 14,
  },
  async (c) => {
    const apiKey = c.authorizationToken;

    // Look up the account using the auth token.
    const item = table.query({
      index: 'GSI1',
      query: {
        expression: 'PK = :pk',
        expressionValues: { ':pk': { S: apiKey } },
      },
    }).items[0];

    // If it exists and it's a valid account, allow request.
    if (item?.entityType === 'Account') {
      const accountId = item.Id;
      return {
        isAuthorized: true,
        // Add account id and api key into the resolver context.
        resolverContext: { accountId, apiKey },
        deniedFields: [],
      };
    }

    //
    return {
      isAuthorized: false,
      resolverContext: {},
      deniedFields: [],
    };
  },
);

And here is the part of the generated code with the __table object.

var __table = {};
var __table_query = {};
__table_query.kind = "Table.query";
__table_query.unhandledContext = __f5;
var __table_query_native = {call: undefined, preWarm: undefined};
__table_query.native = __table_query_native;
__table.query = __table_query;
@thantos
Copy link
Collaborator

thantos commented Jun 12, 2022

Table.* Doesn't work in function right now.

You can use $AWS.DynamoDB.Query()

The reason is table.* Use the app sync contracts right now, we have a ticket to resolve this, but it's non trivial to recreate the next token and other parts of the app sync to dynamo contract.

The larger issue is that lambda doesn't give an error at compile/synth time which I'll look into.

@volkanunsal
Copy link
Contributor Author

Got it. I'll try that. $AWS.DynamoDB.Query() isn't a great option because it returns Dynamo types, which trip up Typescript when discriminating on tagged unions, and I would have to erase the types entirely.

@volkanunsal
Copy link
Contributor Author

I'm getting this error now

CfnSynthesisError: Resolution error: Supplied properties not correct for "CfnFunctionProps"
  environment: supplied properties not correct for "EnvironmentProperty"
    variables: element 'env__functionless315': [{"indexName":"GSI1","keySchema":[{"attributeName":"PK","keyType":"HASH"},{"attributeName":"SK","keyType":"RANGE"}],"projection":{"projectionType":"ALL"}}] should be a string.

@volkanunsal
Copy link
Contributor Author

volkanunsal commented Jun 12, 2022

Is TableName supposed to be a string or a Table? The types tell me it's a Table, but the error message implies that it should be a string.

@sam-goodwin
Copy link
Collaborator

sam-goodwin commented Jun 12, 2022

Is TableName supposed to be a string or a Table? The types tell me it's a Table, but the error message implies that it should be a string.

It supposed to be a reference to a Table. I used the field TableName for consistency with the AWS sdk. But in hindsight, that was likely confusing. We should change it. See #241

The reason it takes a reference is for type safety.

@volkanunsal
Copy link
Contributor Author

@sam-goodwin Do you know how I can work around that error?

@sam-goodwin
Copy link
Collaborator

Pass the table reference as TableName.

Or do you mean the CFN synthesis error? First time seeing that, will take a look soon.

@thantos
Copy link
Collaborator

thantos commented Jun 12, 2022

The closure serializer serializes the entire table object. We attempt to remove unnecessary fields that don't serialize well, but it's a manual process to find them.

After removing unnecessary fields, we try to turn all token into environment variables.

And apparently env variables must eventually resolve to literal strings.

Ran into this on Friday and had not figured out who to detect or normalize yet.

@thantos
Copy link
Collaborator

thantos commented Jun 12, 2022

For now, not using GSIs should bypass the issue. Will prioritize making the token serialization more robust.

@volkanunsal
Copy link
Contributor Author

Even after removing the GSI, I'm still getting that error.

@volkanunsal
Copy link
Contributor Author

Oh, you meant to remove the GSI from the table, I guess. That does bypass the issue.

@sam-goodwin
Copy link
Collaborator

We should also prioritize these two issues. It's clear that new users are going to quickly stumble into these same problems - appsync-only resolvers in lambda and incomplete dynamo index support.

#136

#132

@thantos
Copy link
Collaborator

thantos commented Jun 13, 2022

My list right now is...

Another big breaking change like #132 can follow async.

@thantos
Copy link
Collaborator

thantos commented Jun 13, 2022

Not sure #136 is a huge priority, but testing lambda with tables that have gsi as part of #237 is.

Mostly because the escape hatch is just to wrap a CDK table.

@thantos
Copy link
Collaborator

thantos commented Jun 14, 2022

Closing in favor of

#241
#132
#237

@thantos thantos closed this as completed Jun 14, 2022
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

No branches or pull requests

3 participants