From acda3625de2429db4c500ff0f5b60568623b59d9 Mon Sep 17 00:00:00 2001 From: Brett Prucha Date: Sat, 29 May 2021 23:09:41 -0400 Subject: [PATCH 1/3] Fix bug where on batch error status, the response JSON body wasn't included in the exception On batch changeset, include the content ids, status codes, and response bodies in the result --- src/OBatch.ts | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/src/OBatch.ts b/src/OBatch.ts index 16fcd52..bc6650b 100644 --- a/src/OBatch.ts +++ b/src/OBatch.ts @@ -72,7 +72,7 @@ export class OBatch { const error = await res.json(); throw { res, error }; } catch (ex) { - throw res; + throw typeof ex.error !== 'undefined' ? ex : res; } } } @@ -98,18 +98,27 @@ export class OBatch { dataSegments.shift(); wasWithChangesetresponse = true; return this.parseResponse(dataSegments.join("\r\n\r\n"), header); - } else if (dataSegments.length === 3) { - // if length >= 3 we have a body, try to parse if JSON and return that! - try { - const parsed = JSON.parse(dataSegments[2]); - const hasFragment = parsed[this.batchConfig.fragment]; - return hasFragment || parsed; - } catch (ex) { - return dataSegments[2]; - } } else { - // it seems like we have no body, return the status code - return +dataSegments[1].split(" ")[1]; + var contentIdHeader = dataSegments[0].split("\r\n").find(function (x) { return x.startsWith("Content-ID: "); }); + if (contentIdHeader) { + try { + var contentId = parseInt(contentIdHeader.substring(12), 10); + } catch (ex) { + } + } + var status = +dataSegments[1].split(" ")[1]; + if (dataSegments.length === 3) { + // if length == 3 we have a body, try to parse if JSON and return that! + var body; + try { + const parsed = JSON.parse(dataSegments[2]); + const hasFragment = parsed[this.batchConfig.fragment]; + body = hasFragment || parsed; + } catch (ex) { + body = dataSegments[2]; + } + } + return { contentId, status, body }; } }); if (wasWithChangesetresponse) { From 2068edfac4c321f6ee705a20ad701f82c665f4c7 Mon Sep 17 00:00:00 2001 From: Brett Prucha Date: Sun, 30 May 2021 05:42:45 -0400 Subject: [PATCH 2/3] Allow the caller to process res.json() --- src/OBatch.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/OBatch.ts b/src/OBatch.ts index bc6650b..ee68629 100644 --- a/src/OBatch.ts +++ b/src/OBatch.ts @@ -67,13 +67,7 @@ export class OBatch { const data = await res.text(); return this.parseResponse(data, res.headers.get("Content-Type")); } else { - // check if return is JSON - try { - const error = await res.json(); - throw { res, error }; - } catch (ex) { - throw typeof ex.error !== 'undefined' ? ex : res; - } + throw res; } } From 0f9b65f692f4abc49392ed3612413cc3d88816a0 Mon Sep 17 00:00:00 2001 From: Brett Pruha Date: Mon, 31 May 2021 18:31:41 -0400 Subject: [PATCH 3/3] Update batch unit tests --- src/o.spec.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/o.spec.ts b/src/o.spec.ts index 5d344be..1101f8c 100644 --- a/src/o.spec.ts +++ b/src/o.spec.ts @@ -454,7 +454,7 @@ describe("Batching", () => { const data = await oHandler.get(resource1).get(resource2).batch(); // expect expect(data.length).toBe(2); - expect(data[0].length).toBeDefined(); + expect(data[0].body.length).toBeDefined(); }); test("Batch multiple GET requests and allow to add a query", async () => { @@ -466,7 +466,7 @@ describe("Batching", () => { .get(resource2) .batch({ $top: 2 }); // expect - expect(data[0].length).toBe(2); + expect(data[0].body.length).toBe(2); }); test("Batch multiple GET requests and patch something", async () => { @@ -480,8 +480,8 @@ describe("Batching", () => { .batch(); // expect expect(data.length).toBe(3); - expect(data[1]).toBe(204); - expect(data[2].Name).toBe("New"); + expect(data[1].status).toBe(204); + expect(data[2].body.Name).toBe("New"); }); test("Batch POST and PATCH with useChangeset=true", async () => { @@ -502,8 +502,8 @@ describe("Batching", () => { const data = await request.batch(); // expect expect(data.length).toBe(2); - expect(data[0].LastName).toBe(resouce1data.LastName); - expect(data[1]).toBe(204); + expect(data[0].body.LastName).toBe(resouce1data.LastName); + expect(data[1].status).toBe(204); }); // Content ID seems to have a problem in the test implementation (or I don't get the right implementation) @@ -525,7 +525,7 @@ describe("Batching", () => { .batch(); // expect expect(result.length).toBe(3); - expect(result[1]).toBe(204); - expect(result[2].LastName).toBe("Bar"); + expect(result[1].status).toBe(204); + expect(result[2].body.LastName).toBe("Bar"); }); });