AleksandrRogov / DynamicsWebApi

DynamicsWebApi is a Microsoft Dataverse Web API helper library for JavaScript & TypeScript
MIT License
268 stars 58 forks source link

`continueOnError` config parameter does not work as expected #178

Closed albertjannap closed 1 month ago

albertjannap commented 1 month ago

DynamicsWebApi version v9.2

Describe the bug I'm using the batch functionality to import records into a Dynamics org and I want to use the continueOnError config paramater. I set it to true but my request still fails. I also tried to use the headers paramater to pass the Prefer header with the value odata.continue-on-error but that also didn't work.

Expected behavior I don't want the executbeBatch method to throw an error, the error should show in the results of the executbeBatch response. I tried to import 300 records with 3 records for which I expect an error so 297 records should be imported.

Actual result image Throws an error and my application crashes.

Code Snippet

dynamicsApiClient.startBatch();

for (const record of jsonData) {
    await dynamicsApiClient.create({
        data: record,
        collection: collectionName,
        returnRepresentation: true,
        select: headers,
    });
}

try {
    const res: any[] = await dynamicsApiClient.executeBatch({
        continueOnError: true,
        headers: {Prefer: 'odata.continue-on-error'},
    });
    console.log(res);
} catch (e) {
    console.log(e);
}

Your Setup (please complete the following information):

AleksandrRogov commented 1 month ago

hi @albertjannap thank you for submitting the issue. I will need to investigate this.

albertjannap commented 1 month ago

Some records were processed, for example

  {
    '@odata.context': 'https://e-envaj-import-1721145565.crm4.dynamics.com/api/data/v9.2/$metadata#accounts(address1_name,address2_name,name,address1_line1,address1_city,address1_country,address1_stateorprovince,address1_postalcode,address1_telephone2,address1_telephone1,emailaddress1,websiteurl)/$entity',
    '@odata.etag': 'W/"6895922"',
    address1_name: 'Ashlyn',
    address2_name: 'Pinilla',
    name: 'Art Crafters',
    address1_line1: '703 Beville Rd',
    address1_city: 'Opa Locka',
    address1_country: 'Miami-Dade',
    address1_stateorprovince: 'FL',
    address1_postalcode: '33054',
    address1_telephone2: '305-670-9628',
    address1_telephone1: '305-857-5489',
    emailaddress1: 'apinilla@cox.net',
    websiteurl: 'http://www.artcrafters.com',
    accountid: 'ad6f0291-2248-ef11-bfe2-000d3a21c945',
    address1_composite: '703 Beville Rd\r\nOpa Locka, FL 33054\r\nMiami-Dade',
    oDataContext: 'https://e-envaj-import-1721145565.crm4.dynamics.com/api/data/v9.2/$metadata#accounts(address1_name,address2_name,name,address1_line1,address1_city,address1_country,address1_stateorprovince,address1_postalcode,address1_telephone2,address1_telephone1,emailaddress1,websiteurl)/$entity'
  }

I created the records for the account entity and for 3 records I set the postal code to 25 characters to test error handling for batch processing

AleksandrRogov commented 1 month ago

@albertjannap hi again! For continue on error to work as you expect it, you will need to make all operations in a batch non-atomic (separate/outside of a changeset), you can do it the following way:

const res: any[] = await dynamicsApiClient.executeBatch({
  continueOnError: true,
  inChangeSet: false //<- this must be set, otherwise the whole atomic operation (changeset) will be reverted back and therefore execution stops
});

There is also a weird OOTB Dynamics 365 Web API bug that will prevent the last CUD operations to be properly executed. A workaround to it is to make any retrieval (GET) operation at the end. here's the full code that should work for you:

dynamicsApiClient.startBatch();

for (const record of jsonData) {
  //you don't really need await inside batch operation; but it's not a mistake to have it
  await dynamicsApiClient.create({
    data: record,
    collection: collectionName,
    returnRepresentation: true,
    select: headers,
  });
}

//a workaround for a weird ootb D365 Web API bug:
dynamicsWebApi.retrieveMultiple({
  collection: collectionName,
  top: 1, //limiting to a single record
  //select:   <--- I would advise to put a field here, it can just be the primaryid of the table; otherwise it will pull all fields
});

//without this workaround last create operation will fail with a "System.ArgumentException: Stream was not readable" error
//you can try and see for yourself without a workaround, so you are aware what to expect

try {
  const res: any[] = await dynamicsApiClient.executeBatch({
    continueOnError: true,
    inChangeSet: false //making an operation non-atomic
  });

  //just remember that last array item in 'res' will be a retrieval operation
  console.log(res);
} catch (e) {
  console.log(e);
}
albertjannap commented 1 month ago

@AleksandrRogov thx for your response:) the fix works and indeed the workaround is needed. Typical D365 behaviour... I will close the issue

albertjannap commented 1 month ago

@AleksandrRogov you said it is not a mistake to add await inside a batch operation but I does not work for me if I add await, I have to remove it so maybe nice to specify this somewhere or investigate it

AleksandrRogov commented 1 month ago

@albertjannap what happens when you add it? maybe it's because your await is inside for...of loop? I think that could be the problem, haven't thought of that. usually if you want to await multiple actions you would do an array of promises and then await Promise.all(...), like:

const promises = [];
for (const obj of array){
  promises.push(dynamicsWebApi.create(....));
}
await Promise.all(promises);

await is not needed here, because when Batch operation is started with startBatch, all requests are being memorized and executed once you call executeBatch. I should probably mention that as you recommend.

And I am glad that the problem is fixed for you!

albertjannap commented 1 month ago

@AleksandrRogov If I add it I still got the error but when I remove the await the request got handled as I expected, errors are ignored:)

Ah yes using Promise.all is indeed a best practice