From 80b6a1af0d02309167788ed7a95aa1f275e07653 Mon Sep 17 00:00:00 2001 From: spacejack Date: Mon, 13 Nov 2017 11:08:54 -0500 Subject: [PATCH] feat: Don't reject m.request Promise if extract callback supplied (#2006) --- docs/change-log.md | 1 + docs/request.md | 6 ++++-- request/request.js | 2 +- request/tests/test-request.js | 30 ++++++++++++++++++++++++++++++ 4 files changed, 36 insertions(+), 3 deletions(-) diff --git a/docs/change-log.md b/docs/change-log.md index 087202a9..28aabf32 100644 --- a/docs/change-log.md +++ b/docs/change-log.md @@ -20,6 +20,7 @@ - API: `m.redraw()` is always asynchronous ([#1592](https://github.com/MithrilJS/mithril.js/pull/1592)) - API: `m.mount()` will only render its own root when called, it will not trigger a `redraw()` ([#1592](https://github.com/MithrilJS/mithril.js/pull/1592)) - API: Assigning to `vnode.state` (as in `vnode.state = ...`) is no longer supported. Instead, an error is thrown if `vnode.state` changes upon the invocation of a lifecycle hook. +- API: `m.request` will no longer reject the Promise on server errors (eg. status >= 400) if the caller supplies an `extract` callback. This gives applications more control over handling server responses. #### News diff --git a/docs/request.md b/docs/request.md index fe962e50..fb013df0 100644 --- a/docs/request.md +++ b/docs/request.md @@ -54,7 +54,7 @@ Argument | Type | Required | Descr `options.type` | `any = Function(any)` | No | A constructor to be applied to each object in the response. Defaults to the [identity function](https://en.wikipedia.org/wiki/Identity_function). `options.serialize` | `string = Function(any)` | No | A serialization method to be applied to `data`. Defaults to `JSON.stringify`, or if `options.data` is an instance of [`FormData`](https://developer.mozilla.org/en/docs/Web/API/FormData), defaults to the [identity function](https://en.wikipedia.org/wiki/Identity_function) (i.e. `function(value) {return value}`). `options.deserialize` | `any = Function(string)` | No | A deserialization method to be applied to the `xhr.responseText`. Defaults to a small wrapper around `JSON.parse` that returns `null` for empty responses. If `extract` is defined, `deserialize` will be skipped. -`options.extract` | `any = Function(xhr, options)` | No | A hook to specify how the XMLHttpRequest response should be read. Useful for processing response data, reading headers and cookies. By default this is a function that returns `xhr.responseText`, which is in turn passed to `deserialize`. If a custom `extract` callback is provided, the `xhr` parameter is the XMLHttpRequest instance used for the request, and `options` is the object that was passed to the `m.request` call. Additionally, `deserialize` will be skipped and the value returned from the extract callback will not automatically be parsed as JSON. +`options.extract` | `any = Function(xhr, options)` | No | A hook to specify how the XMLHttpRequest response should be read. Useful for processing response data, reading headers and cookies. By default this is a function that returns `xhr.responseText`, which is in turn passed to `deserialize`. If a custom `extract` callback is provided, the `xhr` parameter is the XMLHttpRequest instance used for the request, and `options` is the object that was passed to the `m.request` call. Additionally, `deserialize` will be skipped and the value returned from the extract callback will be left as-is when the promise resolves. Furthermore, when an extract callback is provided, exceptions are *not* thrown when the server response status code indicates an error. `options.useBody` | `Boolean` | No | Force the use of the HTTP body section for `data` in `GET` requests when set to `true`, or the use of querystring for other HTTP methods when set to `false`. Defaults to `false` for `GET` requests and `true` for other methods. `options.background` | `Boolean` | No | If `false`, redraws mounted components upon completion of the request. If `true`, it does not. Defaults to `false`. **returns** | `Promise` | | A promise that resolves to the response data, after it has been piped through the `extract`, `deserialize` and `type` methods @@ -81,6 +81,8 @@ A call to `m.request` returns a [promise](promise.md) and triggers a redraw upon By default, `m.request` assumes the response is in JSON format and parses it into a Javascript object (or array). +If the HTTP response status code indicates an error, the returned Promise will be rejected. Supplying an extract callback will prevent the promise rejection. + --- ### Typical usage @@ -426,7 +428,7 @@ m.request({ ### Retrieving response details -By default Mithril attempts to parse a response as JSON and returns `xhr.responseText`. It may be useful to inspect a server response in more detail, this can be accomplished by passing a custom `options.extract` function: +By default Mithril attempts to parse `xhr.responseText` as JSON and returns the parsed object. It may be useful to inspect a server response in more detail and process it manually. This can be accomplished by passing a custom `options.extract` function: ```javascript m.request({ diff --git a/request/request.js b/request/request.js index 7e4ec744..b5190ba7 100644 --- a/request/request.js +++ b/request/request.js @@ -88,7 +88,7 @@ module.exports = function($window, Promise) { if (xhr.readyState === 4) { try { var response = (args.extract !== extract) ? args.extract(xhr, args) : args.deserialize(args.extract(xhr, args)) - if ((xhr.status >= 200 && xhr.status < 300) || xhr.status === 304 || FILE_PROTOCOL_REGEX.test(args.url)) { + if (args.extract !== extract || (xhr.status >= 200 && xhr.status < 300) || xhr.status === 304 || FILE_PROTOCOL_REGEX.test(args.url)) { resolve(cast(args.type, response)) } else { diff --git a/request/tests/test-request.js b/request/tests/test-request.js index 94e7e172..5ad5da91 100644 --- a/request/tests/test-request.js +++ b/request/tests/test-request.js @@ -519,5 +519,35 @@ o.spec("xhr", function() { o(e instanceof Error).equals(true) }).then(done) }) + o("does not reject on status error code when extract provided", function(done) { + mock.$defineRoutes({ + "GET /item": function() { + return {status: 500, responseText: JSON.stringify({message: "error"})} + } + }) + xhr({ + method: "GET", url: "/item", + extract: function(xhr) {return JSON.parse(xhr.responseText)} + }).then(function(data) { + o(data.message).equals("error") + done() + }) + }) + o("rejects on error in extract", function(done) { + mock.$defineRoutes({ + "GET /item": function() { + return {status: 200, responseText: JSON.stringify({a: 1})} + } + }) + xhr({ + method: "GET", url: "/item", + extract: function() {throw new Error("error")} + }).catch(function(e) { + o(e instanceof Error).equals(true) + o(e.message).equals("error") + }).then(function() { + done() + }) + }) }) })