MithrilJS / mithril.js

A JavaScript Framework for Building Brilliant Applications
https://mithril.js.org
MIT License
13.95k stars 927 forks source link

completeErrorResponse function should stringify response before assigning it to Error in request #2802

Open boazblake opened 1 year ago

boazblake commented 1 year ago

Mithril.js version: 2.2.2 Browser and OS: Chrome, Safari

The BAAS that I am using is sending an error object response that is not in the way completeErrorResponse is expecting.

It seems like completeErrorResponse is expecting the response to be a string but in my case the response is an object. Since new Error expects a string, wh3en completeErrorResponse assigns the error response to new error, it is no longer accessable, as when attempting to parse it, I get [object Object]

Screen Shot 2022-09-17 at 2 50 19 PM Screen Shot 2022-09-17 at 2 51 14 PM
boazblake commented 1 year ago

On further logging, it seems that there is an exception that is not being handled properly at https://github.com/MithrilJS/mithril.js/blob/645cf663b220614d107cde0d86e3a104db3529fc/request/request.js#L97 This is the error:

responseText: [Exception: DOMException: Failed to read the 'responseText' property from 'XMLHttpRequest': The value is only accessible if the object's 'responseType' is '' or 'text' (was 'json'). at XMLHttpRequest.invokeGetter (<anonymous>:3:28) at xhr.onreadystatechange (http://localhost:3000/node_modules/.vite/deps/chunk-5DDGWUNV.js?v=2df1bbed:1525:19)]

and the value of the response parameter is: responseType: "json"

boazblake commented 1 year ago

I just tested with my changes by pointing my package json to the local, and I am wondering if this is the correct way to handle this issue, as I am now getting a string Error: {code:...., message:...}, so perhaps the rejection should not throw an Error if the response is JSON but the error object itself?

var completeErrorResponse = function () {
  if (ev.target.responseType == "json") {
      reject(response)
  } else {
      try { message = ev.target.responseText }
      catch (e) { message = response }
      var error = new Error(message)
      error.code = ev.target.status
      error.response = response
      reject(error)
  }
}
dead-claudia commented 1 year ago

On further logging, it seems that there is an exception that is not being handled properly at

https://github.com/MithrilJS/mithril.js/blob/645cf663b220614d107cde0d86e3a104db3529fc/request/request.js#L97

This is the error:

responseText: [Exception: DOMException: Failed to read the 'responseText' property from 'XMLHttpRequest': The value is only accessible if the object's 'responseType' is '' or 'text' (was 'json'). at XMLHttpRequest.invokeGetter (<anonymous>:3:28) at xhr.onreadystatechange (http://localhost:3000/node_modules/.vite/deps/chunk-5DDGWUNV.js?v=2df1bbed:1525:19[](http://localhost:3000/node_modules/.vite/deps/chunk-5DDGWUNV.js?v=2df1bbed:1525:19))]

and the value of the response parameter is: responseType: "json"

You're...kidding...

Is there seriously no way to get the original text as a string from an XHR independent of responseType?

boazblake commented 1 year ago

https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/responseText no. If the value of responseType is not "" or "text" you cant read it as text.

I can update the PR to check for the string value of these types? https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/responseType