Display response for multipart content type
Categories
(DevTools :: Netmonitor, defect, P2)
Tracking
(Not tracked)
People
(Reporter: Honza, Assigned: bomsy)
References
(Blocks 2 open bugs)
Details
Attachments
(5 files)
This is a follow up for bug 1611081
As soon as the platform is properly supporting multipart content types - DevTools Network panel backend responsible for collecting HTTP responses - i.e. NetworkResponseListener - should properly collect all parts for mutlipart response (not just the first one).
NetworkResponseListener disconnects the output stream in onStopRequest
, which is the reason why we get only the first part of multi part response.
NetworkResponseListener needs to detect nsIMultiPartChannel (by QI-ing) and keep the stream opened until onAfterLastPart
is executed.
Very similar to what nsStreamListenerTee is doing now.
https://phabricator.services.mozilla.com/D66713#change-3BjiMBRVqXqi
(we can do pretty much the same in NetworkResponseListener
)
See also this comment:
https://bugzilla.mozilla.org/show_bug.cgi?id=1611081#c31
STR:
- Load https://xenon.coheris.com/ff73/
- Inspect the response in the Network panel
Honza
Reporter | ||
Comment 1•4 years ago
|
||
Harald, we can simply concatenate all the responses together (like on the screenshot) or come up with some improved UI.
Any idea that would improve the response-data-inspection workflow?
Honza
Reporter | ||
Updated•4 years ago
|
Comment 2•4 years ago
|
||
:Honza, if you think that's a regression, then could you try to find a regression range in using for example mozregression?
Reporter | ||
Comment 3•4 years ago
|
||
Not regression, this is a follow up for platform fix in bug bug 1611081
Honza
Comment 4•4 years ago
|
||
Any idea that would improve the response-data-inspection workflow?
That seems easiest, assuming that codemirror makes the best of the content mixed with headers.
Does the same UI apply to multipart request payloads?
Reporter | ||
Comment 5•4 years ago
|
||
Yes, attaching an example from multipart/form-data
HTML form data.
Honza
Updated•4 years ago
|
Comment 6•4 years ago
|
||
Clarified that this is for response only. Request already works.
Reporter | ||
Comment 7•4 years ago
|
||
The bug 1611081 has been fixed and I am attaching a screenshot of what's broken here (following STR from comment #0)
The screenshot on the left is showing that we are missing piece of the response body.
Honza
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 8•4 years ago
|
||
The onAfterLastPart listener never gets called
Assignee | ||
Comment 9•4 years ago
|
||
Hey Matt,
I have implemented the onAfterLastPart
function but it is not getting called. From your patch for bug 1611081 , looks like it should be. I might be missing something. If you have a sec, could you pls have a look. I've also pinged you in the patch.
Thanks
Comment 10•4 years ago
|
||
Is it possible that you're running into bug 1638422?
I can't see any other reason why onAfterLastPart wouldn't be forwarded through.
Reporter | ||
Comment 11•4 years ago
|
||
(In reply to Matt Woodrow (:mattwoodrow) from comment #10)
I can't see any other reason why onAfterLastPart wouldn't be forwarded through.
Could the problem be with the SameCOMIdentity
function?
https://searchfox.org/mozilla-central/rev/559b25eb41c1cbffcb90a34e008b8288312fcd25/netwerk/base/nsStreamListenerTee.cpp#102
I am not seeing it in the other callbacks (onStopRequest, onStartRequests) and they work fine.
Honza
Comment 12•4 years ago
|
||
The interface definition for nsIMultiPartChannelListener (in netwerk/base/nsIMultiPartChannel.idl) needs 'scriptable' for JS code to be able to implement it.
Reporter | ||
Comment 13•4 years ago
|
||
(In reply to Matt Woodrow (:mattwoodrow) from comment #12)
The interface definition for nsIMultiPartChannelListener (in netwerk/base/nsIMultiPartChannel.idl) needs 'scriptable' for JS code to be able to implement it.
Ah, great, thanks for filing the bug and providing a patch Matt!
Honza
Comment 14•4 years ago
|
||
Bomsy, with all deps closed, is there more work needed in this bug to make the right responses show?
Assignee | ||
Comment 15•4 years ago
|
||
@harald Yes. i'll just complete the patch.
Assignee | ||
Comment 16•4 years ago
|
||
diff --git a/devtools/server/actors/network-monitor/network-response-listener.js b/devtools/server/actors/network-monitor/network-response-listener.js
--- a/devtools/server/actors/network-monitor/network-response-listener.js
+++ b/devtools/server/actors/network-monitor/network-response-listener.js
@@ -161,6 +161,9 @@ NetworkResponseListener.prototype = {
*/
request: null,
+ part: null,
+ isLastPart: false,
+
/**
* Set the async listener for the given nsIAsyncInputStream. This allows us to
* wait asynchronously for any data coming from the stream.
@@ -195,8 +198,14 @@ NetworkResponseListener.prototype = {
onDataAvailable: function(request, inputStream, offset, count) {
this._findOpenResponse();
const data = NetUtil.readInputStreamToString(inputStream, count);
+ this.bodySize += count;
- this.bodySize += count;
+ if (this.isMultiPart) {
+ //console.log("onDataAvailable >> ", data, " >> count", count, offset);
+ console.log("Part id >> ", this.part);
+ console.log("Request >> ", this.request);
+ console.log("Last part >> ", this.isLastPart);
+ }
if (!this.httpActivity.discardResponseBody) {
const limit = Services.prefs.getIntPref(
@@ -230,6 +239,10 @@ NetworkResponseListener.prototype = {
}
this.isMultiPart = request instanceof Ci.nsIMultiPartChannel;
// onStartRequest
+ if (this.isMultiPart) {
+ this.part = request.partID;
+ this.isLastPart = request.isLastPart;
+ }
this.request = request;
this._getSecurityInfo();
@@ -387,6 +400,7 @@ NetworkResponseListener.prototype = {
onAfterLastPart: function(status) {
// this does not get called
+ console.log("onAfterLastPart >> ", status);
this.sink.outputStream.close();
},
@@ -583,6 +597,7 @@ NetworkResponseListener.prototype = {
available
);
} else {
+ console.log("on data available called.", this.offset, stream);
this.onDataAvailable(this.request, stream, this.offset, available);
}
}
@Matt I would love your help to clarify a couple of things
I'm trying to get some info about the parts for the MultiPartChannel
but i seem to
- always get
0
as thepartID
for all the parts. - the
isLastPart
is nevertrue
, even for the last part
Also theinputStream
seems to be returning the full content for both parts rather than the relevant content per part.
Are these expected? or am i missing something?
Thanks for your help
Comment 17•4 years ago
|
||
Sorry for the slow response!
(In reply to Hubert Boma Manilla (:bomsy) from comment #16)
I'm trying to get some info about the parts for the
MultiPartChannel
but i seem to
- always get
0
as thepartID
for all the parts.- the
isLastPart
is nevertrue
, even for the last part
These are because the onStartRequest implementation records the first 'request' object in this.request, and then returns early for future onStartRequest calls.
onDataAvailable isn't called by platform, but is synthesized within network-response-listener.js (from onInputStreamReady) using this.request, so it's always called with the request object for the first part.
Also the
inputStream
seems to be returning the full content for both parts rather than the relevant content per part.Are these expected? or am i missing something?
Hmm, this is more concerning.
It looks like the interface nsIStreamListenerTree takes all onDataAvailable calls and copies the data into the stream object provided to it from init. onStart/StopRequest get forwarded to the observer (network-response-listener), but onDataAvailable isn't forwarded, the observer just reads (asynchronously) from the stream it provided.
Given that, it seems expected that the stream would contain all the parts concatenated together again, which probably isn't super useful for you.
Comment 18•4 years ago
|
||
Comment 19•4 years ago
|
||
Baku, do you have any ideas on the best way to fix this?
It just created D81482, which has an extension to the nsIStreamListenerTee interface that is workable for multi-part responses, but I don't really know enough about the streams API to say whether it's a good idea.
Comment 20•4 years ago
|
||
It just created D81482, which has an extension to the nsIStreamListenerTee interface that is workable for multi-part responses, but I don't really know enough about the streams API to say whether it's a good idea.
This seems a totally reasonable approach. Just to be sure, let's ask mayhemer too.
Comment 21•4 years ago
|
||
Sorry for late reply. The patches seem to be fine.
Updated•2 years ago
|
Description
•