Skip to content

fix: Errors from asynchronous API calls have incomplete stack traces#719

Open
ThomasGreiner wants to merge 1 commit into
mozilla:masterfrom
ThomasGreiner:ext_308-stacktrace
Open

fix: Errors from asynchronous API calls have incomplete stack traces#719
ThomasGreiner wants to merge 1 commit into
mozilla:masterfrom
ThomasGreiner:ext_308-stacktrace

Conversation

@ThomasGreiner

Copy link
Copy Markdown

We've been observing various browser errors in the Adblock Plus extension that the browser exposes via browser.runtime.lastError. Unfortunately, the stack traces of those errors terminate at webextension-polyfill (in its callback function that handles browser.runtime.lastError) and therefore don't provide any further information to the extension about which of its API calls led to the error.

e.g. browser.action.setBadgeText({tabId: 123, text: ""})

This pull request contains the following changes to provide a more useful/complete stack trace for such browser errors:

  • Added a new error object to capture the stack trace leading up to an asynchronous API call.
    Error message is based on MDN's description of browser.runtime.lastError.
  • Assign browser.runtime.lastError as message to the new error object.
  • Updated tests accordingly.

Since the Errors "cause" property isn't widely supported yet, overwriting the error's "message" property seems to be next-best approach in order to avoid merging stack traces, as that can get messy. That being said, I acknowledge that this is merely a workaround, and that any approach that allows us to natively retain the full stack trace across such asynchronous API calls should be preferable.

@alexm92

alexm92 commented Mar 17, 2025

Copy link
Copy Markdown

Hey, can we please merge these changes? 🙏

FYI: The CI failed for a different reason, unrelated to code changes.

@rpl

rpl commented Mar 27, 2025

Copy link
Copy Markdown
Member

@ThomasGreiner would you mind to rebase this PR? we fixed the unrelated issue hit by the CI job as part of #722 and we just merged that.

@codecov

codecov Bot commented Mar 28, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.84%. Comparing base (ff0f24b) to head (d31fc60).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #719      +/-   ##
==========================================
+ Coverage   90.71%   90.84%   +0.13%     
==========================================
  Files           1        1              
  Lines         140      142       +2     
==========================================
+ Hits          127      129       +2     
  Misses         13       13              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ThomasGreiner

Copy link
Copy Markdown
Author

@rpl Sure thing, done. Thanks for helping fix the CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants