Bug Fix for responseType = 'json' (responseData)

[cyfung1031]

close #117; close #93

Bug Description

Error throws when responseType is json. See #117 #93

Root Cause

responseData was wrongly set to xhrProxy.responseText for responseType === 'json'

var responseData = !responseType || responseType === 'text' || responseType === 'json' ?		
      xhrProxy.responseText : xhrProxy.response;

Detail

responseType: json is first introduced by Blink in Dec 2011. (Source)
responseType: moz-json was added in Fx 9 and then removed in Fx 10.

As the type suggested, json means the response data is already JSON object, and such that xhrProxy.response shall be used in the response. (i.e. no need to JSON.parse(responseText))

Starting from 2020 (Source: TBC), responseText will no longer be available in Blink, and calling getter of xhr.responseText will directly throw an Error.

---

responseType: json 首次由Blink在2011年12月引入。(来源)
responseType: moz-json 在Fx 9中添加，然后在Fx 10中被移除。

正如其名称所示，json 表示响应数据已经是JSON对象，因此应在响应中使用 xhrProxy.response。（即不需要 JSON.parse(responseText)）

从2020年开始（来源：待定），在Blink中将不再提供 responseText，调用 xhr.responseText 的getter将直接抛出错误。

Impact

The response will be changed to JSON object instead of text if the responseType is json.
This will change the onResponse(response, handler) that the response.response is json not text.
If we are keeping the old behavior, response = JSON.stringify(responseData) in handleResponse(xhr, xhrProxy) and then jsonResponse = JSON.parse(response.response) in onResponse(response, handler) which is very stupid.

---

如果responseType为json，响应将被更改为JSON对象，而不是文本。
这将会改变onResponse(response, handler)，使得response.response是JSON格式而不是文本。
如果我们要保持旧的行为，在handleResponse(xhr，xhrProxy)中使用response = JSON.stringify(responseData)，然后在onResponse(response，handler)中使用jsonResponse = JSON.parse(response.response)，这是非常愚蠢的。

[cyfung1031]

Combined Distribution Script ajaxhook.js for Testing (PR 119, 120, 121, 122)
https://cdn.jsdelivr.net/gh/cyfung1031/ajax-hook@1ebe48e08108449669290a226e52fc6fbf7ec9ef/dist/ajaxhook.js

[andywang425]

Thanks, it works well for me.

[DAHUIAAAAA]

Hello, I saw that you pushed some PR to fix some bugs. Do you have a testing plan for these commits? If confirmed to be correct, I will review your code. Thank you.

[cyfung1031]

Hello, I saw that you pushed some PR to fix some bugs. Do you have a testing plan for these commits? If confirmed to be correct, I will review your code. Thank you.

I have been used my modified version for these two weeks. You can try by yourself to confirm it works.
https://github.com/cyfung1031/ajax-hook/tree/temp-20230817
https://cdn.jsdelivr.net/gh/cyfung1031/ajax-hook@1ebe48e08108449669290a226e52fc6fbf7ec9ef/dist/ajaxhook.js

All those fixes in the PRs are just few lines. I tried to minimize the changes so that you can easily understand what are the differences.

https://github.com/wendux/ajax-hook/pull/122/files
https://github.com/wendux/ajax-hook/pull/121/files
https://github.com/wendux/ajax-hook/pull/120/files
https://github.com/wendux/ajax-hook/pull/119/files

[DAHUIAAAAA]

使用 try catch 来代替吧，如果其他浏览器有问题，也可以使用这个降级方案

try {
    return JSON.parse(xhrProxy.responseText);
} catch(e) {
    return xhrProxy.response;
}

[cyfung1031]

對於json主流的已經肯定是xhrProxy.response
他有response就一定沒問題
問題是針對IE沒有response，就要靠responseText
這個也只是針對json

response的getter背後是用built-in的array buffer和parse, 比JSON.parse快

• !responseType || responseType === 'text' 是在MDN文件中列明這只text
• 然後，不是text的情況，其他形式的response都是Object類，放在response
• 但有一個奇疤，就是舊IE裡錯誤地把 json 當成text，只放在responseText沒放在response

我明白你的考慮，但這樣try catch不是一個好處理。
按你的意思會變成

      var responseType = xhrProxy.responseType;
      if (!responseType || responseType === 'text') {
        return xhrProxy.responseText;
      }
      if (responseType === 'json') {
        const response = xhrProxy.response;
        if(response) return response;
        try{
           return JSON.parse(xhrProxy.responseText);
        }catch(e){
           return response;
         }
      }
      return xhrProxy.response;

另外JSON.parse本身也有可能被其他script 改過

原本的修改已經考量過不同的方案，是最簡單有效的
同時亂用try catch的話也會有其他問題出現吧
如果現在這個改法有什麼不行，再改也是很簡單
但try catch後有問題就不知道原因了

[DAHUIAAAAA]

直接筛选 ua 是不合理

• 首先，如果之后浏览器厂商之后 fix 掉这个问题，这个时候用 ua 来判断就不正确了
• 其次，ua 同样会有被改写的可能，尤其是在移动端，不同手机机型的 ua 可能会被魔改
• 最后，我看 axios 似乎也面临过同样的问题，可以参考下
  https://github.com/axios/axios/pull/2424/files

[cyfung1031]

已改成不筛选 ua

[DAHUIAAAAA]

这里的作用是？

[cyfung1031]

我留意到

var responseData = !responseType || responseType === 'text' || responseType === 'json' ?		      // object getter is part of ES5
      xhrProxy.responseText : xhrProxy.response;

除了responseType的問題，是不是每一次都要取response也是一個問題
response很大機會是ReadableStream
responseText也只是讀了ReadableStream再封回成text
也就是說這個行為有資源開銷

當然因為xhr都封在裡面，外面不會有二次讀取的問題
但另一個問題是不是所有request裡的response都能讀。有一些response根本是有問題讀不了，然後報錯

此外，使用者不一定讀取response，這就浪費了電腦資源
可以是一個POST request用來提交資源然後再做一堆動作。雖然用了onResponse但本質上不需要讀response，只是檢查status是否302之類

[DAHUIAAAAA]

我理解这个操作相当于访问 response 的缓存，如果已经设置过 response 或者已经被读取过 response，则可以直接抛出缓存过的 response，这个可以节省一定程度的开销，但是我们需要考虑一些问题：

• 首先，这个改动增加了代码行数，增加了代码的理解难度
• 其次，“资源开销”到底能节省多少？是否能忽略不计？照这么来说，创建getter和setter也有一定的开销

我认为还是去掉比较好

[cyfung1031]

查了一下，getter setter的確不用，已去掉

[cyfung1031]

已按要求修改，使用try catch

[DAHUIAAAAA]

不需要 xhrProxy.readyState === 4 && xhrProxy.status !== 0 这个判断，在 stateChangeCallback 这个函数里已经判断了

[cyfung1031]

去掉了