feat: add window / document checks, refactor XMLHttpRequest to Fetch

[KonnorRogers]

Status

** Ready **

Description

Reduce our calls to windows object, use fetch instead of XMLHTTPRequest, and other niceties to allow Chrome Extensions and WebWorkers to use Honeybadger.

Related Issues

#686
#956

Todos

• Tests
• Documentation
• Changelog Entry (unreleased)

Additional notes

In some light testing of cloudflare workers I discovered the following:\

• "document", "window", and the "Location API" are not present.
• catching things like onunhandledpromiserejection doesn't actually work.

Example:

Honeybadger.configure({
 // ...
})

export default {
  async fetch(): Promise<Response> {
     throw Error("im an error")
     return new Response("Hello World!");
  },
};

The above will just straight up error and it doesnt get caught by honeybadger.

There are some docs on handling errors in CF workers, but they feel rather manual and I havent seen anything about catching global errors.

https://developers.cloudflare.com/workers/learning/logging-workers/

Happy to do some more digging, but you can at least call Honeybadger.notify() from a CF worker and it wont error.

export default {
  async fetch(): Promise<Response> {
    Honeybadger.notify()
  },
};

[subzero10]

Hey @KonnorRogers, I started going through the PR (I know it's not ready, just wanted to have an idea) but it's difficulty to follow with all the formatting changes. Can you please remove the changed semi-colons and we can decide on a convention at a different PR?

[KonnorRogers]

@subzero10 Was already looking at fixing the diff to be easier to follow 🙈

[subzero10]

@subzero10 Was already looking at fixing the diff to be easier to follow 🙈

Awesome, thanks! I will come back to this later then 😄

[KonnorRogers]

@subzero10 should be much easier to parse now.

[subzero10]

Great work!
Did you get a chance to also test manually? What did you test with (i.e. browser, nodejs, chrome extension, web worker)?
I think it's necessary here because our integration tests mock the http call, which is something that we are changing with the PR.

[KonnorRogers]

Great work! Did you get a chance to also test manually? What did you test with (i.e. browser, nodejs, chrome extension, web worker)? I think it's necessary here because our integration tests mock the http call, which is something that we are changing with the PR.

I did get a chance to test manually in the following environments:

• Chrome Extension
• Browser
• Web Worker
• Cloudflare worker

I basically tested that Honeybadger.configure() and Honeybadger.notify() didnt produce errors and were able to send reports.

I did not test NodeJS since I didnt touch anything in core or in server.

[subzero10]

Great work! Did you get a chance to also test manually? What did you test with (i.e. browser, nodejs, chrome extension, web worker)? I think it's necessary here because our integration tests mock the http call, which is something that we are changing with the PR.

I did get a chance to test manually in the following environments:

• Chrome Extension
• Browser
• Web Worker
• Cloudflare worker

I basically tested that Honeybadger.configure() and Honeybadger.notify() didnt produce errors and were able to send reports.

I did not test NodeJS since I didnt touch anything in core or in server.

Sounds great! Yeah I don't see a reason to manually test server-side. We are good to go then!

[subzero10]

@KonnorRogers Btw, I think this is more of a feature rather than a fix. What do you think?

[KonnorRogers]

Id agree with feature here.

[KonnorRogers]

Fun reminder of how annoying this problem is with detecting where a library is being called from:

https://twitter.com/TkDodo/status/1598992656226344960

https://twitter.com/alexdotjs/status/1598996196357738497

[thatsmydoing]

Given the switch to using fetch that means IE is fully unsupported, should there be a list of supported browsers / runtimes in the README or documentation?

[subzero10]

Given the switch to using fetch that means IE is fully unsupported, should there be a list of supported browsers / runtimes in the README or documentation?

Actually, there is a page in our docs, but it's outdated (it says that IE 11 is supported).
We will update it and probably create a table with more details on supported versions per browser.

Thank you for pointing it out!