aws-amplify / amplify-js

A declarative JavaScript library for application development using cloud services.
https://docs.amplify.aws/lib/q/platform/js
Apache License 2.0
9.4k stars 2.12k forks source link

DataStore.save warning: "Variable 'input' has coerced Null value for NonNull type 'String!'" #12069

Open JoakimMellonn opened 10 months ago

JoakimMellonn commented 10 months ago

Before opening, please confirm:

JavaScript Framework

Angular

Amplify APIs

DataStore

Amplify Categories

api

Environment information

``` System: OS: macOS 13.5.2 CPU: (10) arm64 Apple M1 Pro Memory: 77.30 MB / 16.00 GB Shell: 5.9 - /bin/zsh Binaries: Node: 18.13.0 - /usr/local/bin/node npm: 9.8.1 - /opt/homebrew/bin/npm Browsers: Chrome: 116.0.5845.187 Safari: 16.6 npmPackages: @angular-devkit/build-angular: ^15.0.0 => 15.2.9 @angular/animations: ~15.0.0 => 15.0.4 @angular/cdk: ~15.0.0 => 15.0.4 @angular/cli: ~15.0.0 => 15.0.5 @angular/common: ~15.0.0 => 15.0.4 @angular/compiler: ~15.0.0 => 15.0.4 @angular/compiler-cli: ~15.0.0 => 15.0.4 @angular/core: ~15.0.0 => 15.0.4 @angular/forms: ~15.0.0 => 15.0.4 @angular/material: ~15.0.0 => 15.0.4 @angular/platform-browser: ~15.0.0 => 15.0.4 @angular/platform-browser-dynamic: ~15.0.0 => 15.0.4 @angular/router: ~15.0.0 => 15.0.4 @angular/service-worker: ~15.0.0 => 15.0.4 @dicebear/collection: ^6.0.4 => 6.0.4 @dicebear/core: ^6.0.4 => 6.0.4 @ffmpeg/core: ^0.11.0 => 0.11.0 @ffmpeg/ffmpeg: ^0.11.6 => 0.11.6 @stripe/stripe-js: ^1.35.0 => 1.54.2 @types/file-saver: ^2.0.5 => 2.0.5 @types/jasmine: ~3.10.0 => 3.10.13 @types/node: ^12.11.1 => 12.20.55 (18.17.17) aws-amplify: ^5.3.11 => 5.3.11 chart.js: ^4.4.0 => 4.4.0 chart.js-auto: undefined () chart.js-helpers: undefined () docx: ^7.7.0 => 7.8.2 ini: ^1.3.5 => 1.3.8 (3.0.1) inquirer: ^6.5.1 => 6.5.2 (8.2.4) jasmine-core: ~4.0.0 => 4.0.1 (3.99.1) karma: ~6.3.0 => 6.3.20 karma-chrome-launcher: ~3.1.0 => 3.1.1 karma-coverage: ~2.1.0 => 2.1.1 karma-coverage-coffee-example: 1.0.0 karma-jasmine: ~4.0.0 => 4.0.2 karma-jasmine-html-reporter: ~1.7.0 => 1.7.0 ngx-filesaver: ^14.0.0 => 14.0.0 node-html-parser: ^6.1.4 => 6.1.10 rxjs: ~7.5.0 => 7.5.7 (6.6.7, 7.8.1) rxjs/ajax: undefined () rxjs/fetch: undefined () rxjs/internal-compatibility: undefined () rxjs/operators: undefined () rxjs/testing: undefined () rxjs/webSocket: undefined () tslib: ^2.4.1 => 2.6.2 (1.14.1, 2.5.0) typescript: ~4.8.4 => 4.8.4 webpack-cli: ^4.10.0 => 4.10.0 zone-mix: undefined () zone-node: undefined () zone-testing: undefined () zone.js: ~0.11.4 => 0.11.8 zone.js/async-stack-tagging: undefined () zone.js/async-stack-tagging.min: undefined () zone.js/async-test: undefined () zone.js/async-test.min: undefined () zone.js/fake-async-test: undefined () zone.js/fake-async-test.min: undefined () zone.js/jasmine-patch: undefined () zone.js/jasmine-patch.min: undefined () zone.js/long-stack-trace-zone: undefined () zone.js/long-stack-trace-zone.min: undefined () zone.js/mocha-patch: undefined () zone.js/mocha-patch.min: undefined () zone.js/proxy: undefined () zone.js/proxy.min: undefined () zone.js/sync-test: undefined () zone.js/sync-test.min: undefined () zone.js/task-tracking: undefined () zone.js/task-tracking.min: undefined () zone.js/webapis-media-query: undefined () zone.js/webapis-media-query.min: undefined () zone.js/webapis-notification: undefined () zone.js/webapis-notification.min: undefined () zone.js/webapis-rtc-peer-connection: undefined () zone.js/webapis-rtc-peer-connection.min: undefined () zone.js/webapis-shadydom: undefined () zone.js/webapis-shadydom.min: undefined () zone.js/wtf: undefined () zone.js/wtf.min: undefined () zone.js/zone-bluebird: undefined () zone.js/zone-bluebird.min: undefined () zone.js/zone-error: undefined () zone.js/zone-error.min: undefined () zone.js/zone-legacy: undefined () zone.js/zone-legacy.min: undefined () zone.js/zone-patch-canvas: undefined () zone.js/zone-patch-canvas.min: undefined () zone.js/zone-patch-cordova: undefined () zone.js/zone-patch-cordova.min: undefined () zone.js/zone-patch-electron: undefined () zone.js/zone-patch-electron.min: undefined () zone.js/zone-patch-fetch: undefined () zone.js/zone-patch-fetch.min: undefined () zone.js/zone-patch-jsonp: undefined () zone.js/zone-patch-jsonp.min: undefined () zone.js/zone-patch-message-port: undefined () zone.js/zone-patch-message-port.min: undefined () zone.js/zone-patch-promise-test: undefined () zone.js/zone-patch-promise-test.min: undefined () zone.js/zone-patch-resize-observer: undefined () zone.js/zone-patch-resize-observer.min: undefined () zone.js/zone-patch-rxjs: undefined () zone.js/zone-patch-rxjs-fake-async: undefined () zone.js/zone-patch-rxjs-fake-async.min: undefined () zone.js/zone-patch-rxjs.min: undefined () zone.js/zone-patch-socket-io: undefined () zone.js/zone-patch-socket-io.min: undefined () zone.js/zone-patch-user-media: undefined () zone.js/zone-patch-user-media.min: undefined () npmGlobalPackages: create-react-app: 5.0.1 firebase-tools: 12.5.4 gatsby-cli: 5.3.1 nativescript: 8.5.3 neovim: 4.10.1 npm: 9.8.1 typewritten: 1.5.1 vscode-langservers-extracted: 4.6.0 ```

Describe the bug

I'm getting this warning when saving a Datastore object, after using obj.copyOf() to update one value on the object. I don't know when, but it stopped working some time ago, and I haven't been able to find a workaround or a fix, so this exact code has worked earlier. And what I think the weird thing is, is the fact that the "newRecording" changes from when I console.log it to when I save it in Datastore.

Expected behavior

The object should be updated accordingly and saved to Datastore without warning.

Reproduction steps

  1. Create a js web app
  2. Create a Datastore object and update it with obj.copyOf()
  3. Save it to Datastore

Code Snippet

const recording = new Recording({
  name: title,
  description: desc,
  date: new Date().toISOString(),
  fileName: file.name,
  fileKey: '',
  speakerCount: speakerCount,
  languageCode: languageCode
});
...
const key = 'recordings/' + recording.id + '.' + fileType;
const newRecording = Recording.copyOf(recording, copy => {
  copy.fileKey = key
});

try {
  console.log(newRecording);
  await DataStore.save(newRecording);
  ...

Log output

This is the warning: ``` tslib:71 [WARN] 58:16.393 DataStore {recoverySuggestion: 'Ensure app code is up to date, auth directives exi… https://github.com/aws-amplify/amplify-js/issues', localModel: date: undefined description: undefined fileKey: "recordings/48cced19-cf0c-49aa-86a2-0c9a448a1343.mp3" fileName: undefined fileUrl: undefined id: "48cced19-cf0c-49aa-86a2-0c9a448a1343" interviewers: undefined labels: undefined languageCode: undefined name: undefined speakerCount: undefined _version: undefined message: "Variable 'input' has coerced Null value for NonNull type 'String!'", operation: 'Create', errorType: 'BadRecord', …} cause: locations: [{…}] message: "Variable 'input' has coerced Null value for NonNull type 'String!'" path: null [[Prototype]]: Object errorInfo: undefined errorType: "BadRecord" localModel: {id: '03d04c19-0b4d-4053-a553-cdc6fe36cbc6', name: undefined, date: undefined, description: undefined, fileKey: 'recordings/03d04c19-0b4d-4053-a553-cdc6fe36cbc6.mp3', …} message: "Variable 'input' has coerced Null value for NonNull type 'String!'" operation: "Create"process: "mutate" recoverySuggestion: "Ensure app code is up to date, auth directives exist and are correct on each model, and that server-side data has not been invalidated by a schema change. If the problem persists, search for or create an issue: https://github.com/aws-amplify/amplify-js/issues" remoteModel: null [[Prototype]]: Object ``` This is the log from printing the "newRecording" before saving it: ``` createdAt: null date: "2023-09-16T15:28:04.732Z" description: "Purchase tracking" fileKey: "recordings/48cced19-cf0c-49aa-86a2-0c9a448a1343.mp3" fileName: "Proshop Test.mp3" fileUrl: null id: "48cced19-cf0c-49aa-86a2-0c9a448a1343" interviewers: null labels: null languageCode: "da-DK" name: "Purchase test5" speakerCount: 2 updatedAt: null _deleted: undefined _lastChangedAt: undefined _version: undefined ```

aws-exports.js

No response

Manual configuration

No response

Additional configuration

No response

Mobile Device

No response

Mobile Operating System

No response

Mobile Browser

No response

Mobile Browser Version

No response

Additional information and screenshots

No response

cwomack commented 10 months ago

Hello, @JoakimMellonn and thanks for creating this issue. Are you performing a save of the new Recording in that first line of example code? Can you also share your Amplify GraphQL schema? Thanks.

JoakimMellonn commented 10 months ago

Hello, @JoakimMellonn and thanks for creating this issue. Are you performing a save of the new Recording in that first line of example code? Can you also share your Amplify GraphQL schema? Thanks.

I don't save the one called "recording" in Datastore, I just create it to get an ID and then copy it to put in the fileKey. I have found a workaround by saving it in Datastore before making the copy, but I don't see why this should be necessary.

The GraphQL schema for the Recording type is here (if more is needed please do let me know):

type Recording @model @auth(rules: [{allow: owner}]) {
  id: ID!
  name: String!
  date: AWSDateTime
  description: String
  fileKey: String
  fileName: String
  fileUrl: String
  speakerCount: Int!
  languageCode: String
  versions: [Version] @connection(keyName: "byRecording", fields: ["id"])
  interviewers: [String]
  labels: [String]
}
cwomack commented 10 months ago

@JoakimMellonn, appreciate the clarity and you sharing your schema. It look like you could either save the new record beforehand OR just annotate the ID field with @primaryKey, which will allow you to manage it yourself.

You could then import in the uuid library and set the id field manually. The uuid library comes with DataStore, so it doesn't need to be installed separately (as seen in the code example below). This way you can accomplish your intended use case with a single save.

import { v4 } from 'uuid';

const id = v4();
const key = 'recordings/' + id + '.' + fileType;

const recording = new Recording({
  name: title,
  description: desc,
  date: new Date().toISOString(),
  fileName: file.name,
  fileKey: key,
  speakerCount: speakerCount,
  languageCode: languageCode,
});

await DataStore.save(recording);

Let me know if this helps!

JoakimMellonn commented 10 months ago

Thank you, I think that I will continue to use the method mentioned with saving it. Although I still think it's weird that it just fails with a warning when trying to copy it.

JoakimMellonn commented 10 months ago

An update to this issue: I've found that saving the object both before and after updating the fileKey, actually doesn't save it on the second save. It doesn't display any errors and doesn't indicate anything is wrong. But when I look in Amplify Studio at the saved recording, it misses the fileKey value.

I would appreciate it if this was looked at as a bug and not just a General question, I would personally call this, and the original way I did it, unexpected behaviour. Thank you in advance.

sjdeak commented 9 months ago

Agree with @JoakimMellonn , I spent an entire afternoon debugging this. In summary, the issue is "For model instance that is init locally without saving into DataStore, if run copyOf method on it, the updated instance can't be saved into DataStore anymore." @cwomack Please check whether it's a bug or limitation, for bug please fix it and for limitation please mention it in the document, thanks!

Share my case, my model name is "DailyPlan"

type DailyPlan @model @auth(rules: [{allow: owner}]) {
  id: ID!
  date: AWSDate!
  plan: String!
}
const currentDailyPlan = new DailyPlan({ date: "2023-10-23", plan: "test" });
const updatedDailyPlan = DailyPlan.copyOf(currentDailyPlan, updated => { updated.plan = "test 2"; });

await DataStore.save(currentDailyPlan) will success. await DataStore.save(updatedDailyPlan) will fail with warning message. Variable 'input' has coerced Null value for NonNull type 'AWSDate!'"

I think this pattern (creating model first use in UI, updated its content from user input, then save) is quite common for frontend development. Without supporting this pattern, developer needs to define their own model first then transform back into amplify generated models, this causes duplicate model for same data.

sjdeak commented 9 months ago

@cwomack would you have updates?

sjdeak commented 9 months ago

Updated my comments, tended we should fix this limitation.

cwomack commented 9 months ago

@JoakimMellonn, after reviewing this further... I believe the issue here is that we don't support mutating a record before it's saved. You need to do a save then an update.

Is there any chance we can get steps for the reproduction of where you say "saving the object both before and after updating the fileKey, actually doesn't save it on the second save"? We'd like to see how you're doing that second save (or both saves ideally) so we can reproduce. Do you see both network requests for the 1st and 2nd save and do they look as expected? Curious to know if the version number is incrementing between the 2 requests.

cwomack commented 9 months ago

@sjdeak, similar to the above comment... I think you're currently doing this: 1) new TODO (without save) 2) make several copy-of's 3) save

But I believe you'd need to change your flow to be the following: 1) new TODO (with save) 2) make several copy-of's 3) save

Can you try that change and see if that resolves the issue for you? Is there anything we're missing for your use case that doesn't make the initial save an acceptable solution?

JoakimMellonn commented 9 months ago

@JoakimMellonn, after reviewing this further... I believe the issue here is that we don't support mutating a record before it's saved. You need to do a save then an update.

This is okay, but I would then request that it gets supported in the future, I personally think that it would be an intuitive thing to support, maybe that's just my opinion.

Is there any chance we can get steps for the reproduction of where you say "saving the object both before and after updating the fileKey, actually doesn't save it on the second save"? We'd like to see how you're doing that second save (or both saves ideally) so we can reproduce. Do you see both network requests for the 1st and 2nd save and do they look as expected? Curious to know if the version number is incrementing between the 2 requests.

Of course! Here is the way I'm doing it now, which actually works. But only because i query the recording before updating it and then update the queried recording, instead of the one I just saved.

const recording = new Recording({
  name: title,
  description: desc,
  date: new Date().toISOString(),
  fileName: file.name,
  fileKey: '',
  speakerCount: speakerCount,
  languageCode: languageCode
});

try {
  await DataStore.save(recording);
} catch (err) {
  console.error(`Error while saving recording first time: ${err}`);
}

...

try {
  const key = 'recordings/' + recording.id + '.' + fileType;
  const uploadedRecording = (await DataStore.query(Recording, (r) => r.id.eq(recording.id)))[0]; // without getting the recording to then copy it, it doesn't work.
  const updatedRecording = Recording.copyOf(uploadedRecording, copy => {
    copy.fileKey = key
  });

...
chrisbonifacio commented 8 months ago

Hi @JoakimMellonn 👋 just wanted to follow up and see where we're at.

It sounds like you are able to successfully create and update records but I'd like to confirm that you are unblocked after the guidance provided.

If so, I can convert this issue to a feature request for the ability to mutate a record before it's been saved to the local store.

JoakimMellonn commented 8 months ago

Hi @JoakimMellonn 👋 just wanted to follow up and see where we're at.

It sounds like you are able to successfully create and update records but I'd like to confirm that you are unblocked after the guidance provided.

If so, I can convert this issue to a feature request for the ability to mutate a record before it's been saved to the local store.

Hi sorry for the late reply. I can confirm that I'm currently unblocked by this, and thank you.