ember-simple-auth
ember-simple-auth copied to clipboard
Missing migration path for deprecated `responseJSON` and `responseText` property
During typescript migration we've noted that responseJSON and responseText properties returned from OAuth2PasswordGrantAuthenticator are deprecated. However I've overlooked the fact that there's no clear migration path for this.
Originally reported here: https://github.com/mainmatter/ember-simple-auth/pull/2883#issuecomment-2582732477
What are the alternatives for reponseJSON and reponseText? I use these to show the error returned by the backend on login and since the response you return has already been read, it's not an option to use that one.\
Is there any chance you could return a copy of the response (before it was read)? (or copy the response, read that one and send the original)
Draft: https://github.com/mainmatter/ember-simple-auth/pull/2922
This is not what I meant by cloning the response to access the body:
const response = await fetch(url, options);
const text = await response.text();
const cloned = response.clone() as OAuth2Response;
try {
const json = JSON.parse(text);
if (response.ok) {
return json;
} else {
cloned.responseJSON = json;
throw cloned;
}
} catch (SyntaxError) {
cloned.responseText = text;
throw cloned;
}
I was thinking about something like this:
const response = await fetch(url, options);
// Clone before reading, so user can also read this cloned response, without getting error about body being used
const cloned = response.clone() as OAuth2Response;
// Read original response
const text = await response.text();
try {
const json = JSON.parse(text);
if (response.ok) {
return json;
} else {
// We don't need this, the user can just read the cloned response using response.text() or response.json()
// cloned.responseJSON = json;
throw cloned;
}
} catch (SyntaxError) {
// We don't need this, the user can just read the cloned response using response.text() or response.json()
// cloned.responseText = text;
throw cloned;
}
This removes the responseJSON and responseText that normally don't exist on a response + the user can still read the original body using response.text() and response.json() (instead of accessing these properties on the response they read it like any other response).
Unless I'm not catching onto something, the corrected code is pretty much the same right?
I believe that the draft PR does what you're asking for i.e.:
- Adds ability to call
json()andtext()on response. - Adds
responseJSON,responseTextso there's no need to commit a breaking change for now. We'd like to provide migration paths in between major versions ideally.
For now I'd like to try to resolve it without making a breaking change, but if it'll be too cumbersome then we might drop them with the next release.
Your PR is almost correct, you just need to place the clone() before the text() method.
Otherwise, you're cloning an already read response, so the cloned one can also not be read.
I also think it's not possible to clone an already read response, see https://developer.mozilla.org/en-US/docs/Web/API/Response/clone#:~:text=clone()-,throws%20a%20TypeError,-if%20the%20response