Added chrome.debugger to monitor network requests #44
Conversation
Github accurate Stack overflow accurate Theodo -4% error Youtube accurate Trello -13% error
juskek
force-pushed
the
2023/07/18-Amount-of-data-transferred-in-DevTools-is-different-from-amount-recorded-by-extension
branch
from
5121cd1 to
ef06ac5
Compare
src/background/background.ts
Outdated
| { urls: ["<all_urls>"], tabId }, | ||
| ["responseHeaders"] | ||
| ); | ||
| chrome.debugger.attach({ tabId: tabId }, "1.2", function () { |
There was a problem hiding this comment.
What does this debugger.attach do?
There was a problem hiding this comment.
Attaches the chrome debugger to a tab, allows the extension to listen to network requests for that tab :)
| ); | ||
| try { | ||
| await chrome.debugger.detach({ tabId: tabId }); | ||
| } catch (e: unknown) { |
There was a problem hiding this comment.
I think it is always going to be an error -
It's typed as unknown because I don't think typing in catch clauses are allowed yet
There was a problem hiding this comment.
In that case, I'd probably remove the if condition and just type cast it as an Error
src/background/background.ts
Outdated
| sendResponse(true); | ||
| } | ||
| return true; | ||
| }); | ||
|
|
||
| chrome.debugger.onEvent.addListener(function (source, method, params) { |
There was a problem hiding this comment.
General question of the codebase, do we use the function keyword over arrow functions?
There was a problem hiding this comment.
oops, arrow notation is preferred, will cleanup
src/background/background.ts
Outdated
| const { encodedDataLength } = params as { | ||
| encodedDataLength?: number; | ||
| }; |
There was a problem hiding this comment.
Ew type casting - could you not just type the function at the top like so: function (source: unknown, method: string, params: { encodedDataLength?: number}) { ... }
src/background/backgroundHelpers.ts
Outdated
| } catch (e: unknown) { | ||
| if (e instanceof Error) { |
There was a problem hiding this comment.
SImilar error question here
src/background/backgroundHelpers.ts
Outdated
| "Could not establish connection. Receiving end does not exist." | ||
| ) { | ||
| console.warn( | ||
| `Error Caught: ${e}\nIf popup is open and this error is seen in the console, debugging is required.` |
There was a problem hiding this comment.
Debugging is required as soon as you have an error, isn't it? We should maybe consider it the other way around "if popup is closed then the error is ok"
There was a problem hiding this comment.
I can catch this error by checking if (popup is closed && could not establish connection), but checking if the popup is closed requires chrome.runtime.getContexts()
- This is a new api that recently got proposed, and the only example I have found where it is used is here
- The example has
"minimum_chrome_version": "116", while the current stable version is 115, so I think we can leave it for now and catch it properly when 116 is out
juskek
force-pushed
the
2023/07/18-Amount-of-data-transferred-in-DevTools-is-different-from-amount-recorded-by-extension
branch
from
ef06ac5 to
75dc4e1
Compare
src/background/background.ts
Outdated
| const error = e as Error; | ||
| if ( | ||
| error.message === | ||
| `Debugger is not attached to the tab with id: ${tabId}.` | ||
| ) { | ||
| console.warn( | ||
| `Tried to detach debugger from tab (tabId: ${tabId}) when there was none attached. ` | ||
| ); | ||
| return; | ||
| } | ||
| throw e; | ||
| } |
There was a problem hiding this comment.
You can just do inline type casting since you don't use error anywhere else: (e as Error).message === ...
juskek
force-pushed
the
2023/07/18-Amount-of-data-transferred-in-DevTools-is-different-from-amount-recorded-by-extension
branch
from
d998849 to
69b6924
Compare
Previous accuracy with chrome.webRequests:
With chrome.debugger: