Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

access-api stacktraces in sentry are not working #623

Closed
vasco-santos opened this issue Mar 23, 2023 · 11 comments · Fixed by #757
Closed

access-api stacktraces in sentry are not working #623

vasco-santos opened this issue Mar 23, 2023 · 11 comments · Fixed by #757
Milestone

Comments

@vasco-santos
Copy link
Contributor

No description provided.

@alanshaw alanshaw mentioned this issue Mar 27, 2023
23 tasks
@gobengo
Copy link
Contributor

gobengo commented Mar 27, 2023

related: #367
tl;dr - we are a bit constrained in how we do this due to d1 public alpha 'no-bundle'.

these starters from toucan-js seem like a promising direction. https://github.com/robertcepa/toucan-js#examples

@gobengo
Copy link
Contributor

gobengo commented Mar 29, 2023

As long as we use d1 which requires no_bundle=false (or omitted) in wrangler.toml, I think we'd have to do something like this which pushes source maps to sentry via sentry-cli in our deploy pipeline. https://github.com/robertcepa/toucan-js/tree/master/examples/wrangler-basic#wrangler-basic-starter

After removing d1, we may be able to go back to no_bundle=true and use some of the other setups if desired. (but also maybe 'basic' is fine)

@gobengo
Copy link
Contributor

gobengo commented Apr 5, 2023

I just learned some things:

I noticed this issue #254
mentions upgrading toucan-js, which is the sentry library that access-api uses.

  • that 3.0.0 release

    This is a complete rewrite of toucan-js. The goal of this update is to reuse more components from @sentry/core, fix long-standing issues with source maps, and provide starters using various bundlers (esbuild, rollup, vite, webpack).

  • mentions source maps, seems worth trying
  • rn access-api uses toucan-js ^2.7.0, so we aren't yet using the latest and greatest source maps stuff.

So what I will try is upgrading toucan-js to >= 3.x, as suggested by #254

@gobengo
Copy link
Contributor

gobengo commented Apr 5, 2023

gobengo added a commit that referenced this issue Apr 6, 2023
Motivation:
* #623 
* test theory that things will work if we upgrade to toucan-js@3, which
is a rewrite that sends source maps in a new way
@gobengo
Copy link
Contributor

gobengo commented Apr 6, 2023

  • I deployed the toucan-js@3.x upgrade to access-api staging env
  • triggered error on staging using w3cli W3_UPLOAD_SERVICE_DID='did:web:staging.web3.storage' W3_UPLOAD_SERVICE_URL='https://staging.up.web3.storage/' W3_ACCESS_SERVICE_DID='did:web:staging.web3.storage' W3_ACCESS_SERVICE_URL='https://w3access-staging.protocol-labs.workers.dev' DEBUG='*' ./shim.js authorize "example@example.com"
    • example@example.com leads to an error (useful here)
    • error in sentry
      • no stack traces :/ (beyond showing export default worker like before)

      • but I also see a 'Tell us where your source code is' link on that. It asked me to give it a github url to '../src/index.js', so I did https://github.com/web3-storage/w3up/blob/main/packages/access-api/src/index.js Screenshot 2023-04-06 at 1 05 27 PM

      • sentry didn't know about the w3up repo, so I added it https://protocol-labs-it.sentry.io/settings/integrations/github/109727/

      • It made 'open this line with github' work, but it still just takes you to the line for export default worker

@gobengo
Copy link
Contributor

gobengo commented Apr 7, 2023

I've been trying to figure out what to try next, and just noticed:

  • the sentry error still doesn't have a source-map aware stack trace
  • says 6.0.0-staging (99c532a) url
    • in sentry, that release exists and does have 2 source map artifacts
  • what does that sha 99c532a in the release correspond to?
    • if I make a github url for it, something loads 99c532a but it says "This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository."
  • theory:
    • the staging release happens from branch release-please--branches--main--components--access-api managed by release-please-action
    • It appears release-please action, doesn't append new commits to that branch, it force-pushes a new one and removes the old one
    • that's unfortunate, and I'm tempted to fix, but that alone wouldn't explain the issue afaict

@gobengo
Copy link
Contributor

gobengo commented Apr 7, 2023

oooh I should try what @olizilla did here
https://github.com/web3-storage/web3.storage/pull/1033/files#diff-fcc5ef774a5ba3602a1c3478168015fa7b4b4b154887b7d2ed197720f65501f1R57

(It's funny, I was reading this toucan-js issue thread and organically stumbled upon an @olizilla robertcepa/toucan-js#54 (comment) )

@gobengo
Copy link
Contributor

gobengo commented Apr 7, 2023

I should also try not using rewriteFrames as suggested robertcepa/toucan-js#54 (comment)

I did just notice that this error, and the last one too, show more info if I click '...' and then check the 'minified' checkbox on the stack trace. Then I can see that this error is coming from inside a fetch invocaiton.
That makes me think that

gobengo added a commit that referenced this issue Apr 7, 2023
)

Motivation:
*
#623 (comment)
* this was suggested by toucan-js author as no longer being needed in
3.0.0.
gobengo added a commit that referenced this issue Apr 8, 2023
Motivation:
* #623 - see if errors from this show up different than errors from
ucanto HandlerExceptionError
@gobengo
Copy link
Contributor

gobengo commented Apr 8, 2023

I wondered if any part of it was due to the way I was triggering the error that got into sentry, which was a ucanto HandlerExecutionError. (I was thinking maybe someone needed to call Error.captureStackTrace(), I don't think that's needed the way ucanto subclasses Error).

So I added a simple route that will throw an error (as oli had done while testing this type of thing).
That produced this error but did not affect stack trace rendering. :|

minified raw stack trace

Error: this is the result of handling a request at /.debug/error
  at <anonymous>(worker.js:20855:9)
  at Yd.fetch(worker.js:8599:23)
  at Object.fetch(worker.js:20861:22)
  at Object.fetch(worker.js:21072:16)

If I uncheck 'minified'

Error: this is the result of handling a request at /.debug/error
  at <unknown>(../src/index.js:44:16)
  at <unknown>(../src/index.js:44:16)
  at <unknown>(../src/index.js:44:16)
  at <unknown>(../src/index.js:44:16)

Why would this be? (!!!)

Brainstorms (I will think over weekend and ask for advice from @olizilla et al)

Other things worth trying:

@gobengo
Copy link
Contributor

gobengo commented Apr 10, 2023

  • meet with @olizilla to brainstorm which other things to try

  • investigate why sentry error still shows line export default worker even though after my change last friday I'd expect it to be like export default { or show the fetch method on that obj

    • I needed to generate a new error on staging using the change I had deployed. Then it still doesn't show a helpful stack trace, it just shows the end of the inlined exported object.
  • inspect the source map files being generated and sent to sentry

    • I did this a bit with a tool like (I forget which) sourcemaps.io last week. They seemed to work just fine, but I haven't used that tool before, so I should try again and document what I did
    • I retested using this tool and js+js.map from latest staging release. No errors reported.

--

Ideas

  • try using @sentry/esbuild-plugin to send the source maps https://github.com/robertcepa/toucan-js/blob/master/examples/wrangler-esbuild/esbuild.config.js#L15
    • ⚠️ after talking with @olizilla I realized this probably won't help. The underlying problem is that no_bundle=false means that cloudflare bundling is probably generating a bad source map
    • pro: it's what toucan-js 3.0.0 examples use, so it's likely to get the most support from @robertcepa if we followup on this past help
    • con: it's not what our other services do (afiact), so it's making things less like our other services.
      • Though those other services use toucan-js 2.7.0 anyway. so if we scout out a way that works with toucan-js 3.0.0, maybe those other services would benefit

@gobengo
Copy link
Contributor

gobengo commented Apr 10, 2023

gobengo added a commit that referenced this issue Apr 11, 2023
Motivation:
* #623 
* test theory that things will work if we upgrade to toucan-js@3, which
is a rewrite that sends source maps in a new way
gobengo added a commit that referenced this issue Apr 11, 2023
)

Motivation:
*
#623 (comment)
* this was suggested by toucan-js author as no longer being needed in
3.0.0.
gobengo added a commit that referenced this issue Apr 11, 2023
Motivation:
* #623 - see if errors from this show up different than errors from
ucanto HandlerExceptionError
gobengo added a commit that referenced this issue Apr 11, 2023
…tely-defined object in hopes stack traces will work better (#733)

Motivation:
* #623
gobengo added a commit that referenced this issue Apr 14, 2023
…gler bundling (#739)

Motivation:
* #623 
* verify our assumption that we have to use `no_bundle=true` along with
d1.

Experiment
* I wanted to try some new ways of building and whether they would work,
but without risking staging, so I used the 'dev' environment from
wrangler.toml to try some deploys from local.
* I made access-api use of wrangler more like toucan-js@3
[wrangler-basic](https://github.com/robertcepa/toucan-js/blob/master/examples/wrangler-basic/wrangler.toml)
* I did a sample deploy from my laptop to 'dev' workers environment,
with the sentry/toucan `release` called `bengo-dev-0` [sentry errors
here](https://protocol-labs-it.sentry.io/issues/?query=+release%3Abengo-dev-0&referrer=issue-list&statsPeriod=14d)
* trigger error via `GET /.debug/error`
* expectation: this to lead to an in-route error and show up in sentry
with helpful stack traces - because this is what
[example](https://github.com/robertcepa/toucan-js/blob/master/examples/wrangler-basic/wrangler.toml)
implies should work
* trigger error via route that uses d1 binding (e.g. ucanto routes
[triggered via w3up+ucanto observable pointing to dev
env](https://observablehq.com/d/a76545064f82a998))
* expectation: d1 error - because supposedly `no_bundle=false` (or
omitting `no_bundle`) is incompatible with wrangler d1 bindings

Findings:
* omg it works!
* [error triggered by GET /.debug/error (no
d1)](https://protocol-labs-it.sentry.io/issues/4078173859/?query=+release%3Abengo-dev-0&referrer=issue-stream&statsPeriod=14d&stream_index=3)
* [error triggered by using d1 via access protocol via
observable](https://protocol-labs-it.sentry.io/issues/4078175792/?query=is%3Aunresolved+release%3Abengo-dev-0&referrer=issue-stream&statsPeriod=14d&stream_index=2)
* The error did not happen in the `access/authorize` handler. That
worked fine, and I got an email from dev env. The error happened when I
clicked the email (invoking `access/confirm`), when the `access/confirm`
invoacation handler tried to write to d1
  * It got a D1_ERROR, but I could see from the sentry report
> Error: ERROR 9009: SQL prepare error: no such table: delegations_v3
* which makes sense, because `dev` doesn't have latest migrations
applied that created that table. So I provisioned a new d1 database for
this dev env, and ran `npx wrangler --env=dev d1 migrations apply
__D1_BETA__`. This would error every couple migrations, but if I re-ran
it it would make progress and error again (`Internal error [code:
7501]`), but eventually it would succeed at all migrations.
* then I re-triggered things via the observable, get email, click email.
I saw 'email validated'! 🎉
* I continued using the observable to invoke `access/claim` (reads from
D1), and got no error all the way through invoking from second device
<details><img width="578" alt="Screenshot 2023-04-10 at 5 48 41 PM"
src="https://user-images.githubusercontent.com/171782/231026937-25bd396b-8261-4a4e-9c8f-141ef9d2bbf8.png"></details>
gobengo pushed a commit that referenced this issue Apr 14, 2023
🤖 I have created a release *beep* *boop*
---


##
[6.1.0](access-api-v6.0.0...access-api-v6.1.0)
(2023-04-14)


### Features

* change access-api wrangler.toml to be no_bundle=false. use wrangler
bundling ([#739](#739))
([d659516](d659516))


### Bug Fixes

* access-api cli.js build does sourcemap=external instead of true
([#731](#731))
([d1a35b7](d1a35b7))
* add /.debug/error route
([#732](#732))
([2ca5de8](2ca5de8))
* change access-api src/index.js 'export default' to be an
immediately-defined object in hopes stack traces will work better
([#733](#733))
([a509313](a509313)),
closes [#623](#623)
* toucan-js in access-api does not use RewriteFrame integration
([#729](#729))
([eeb90e6](eeb90e6))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Peeja pushed a commit to storacha/upload-service that referenced this issue Jan 17, 2025
bring in new `plan/set` invocation API from `w3up-client` and `access`
Peeja pushed a commit to storacha/upload-service that referenced this issue Jan 17, 2025
🤖 I have created a release *beep* *boop*
---


##
[2.1.0](storacha/w3ui@core-v2.0.2...core-v2.1.0)
(2024-02-01)


### Features

* upgrade `core` dependencies for `plan/set`
([storacha#623](storacha/w3ui#623))
([823b0ef](storacha/w3ui@823b0ef))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Peeja pushed a commit to storacha/upload-service that referenced this issue Jan 29, 2025
bring in new `plan/set` invocation API from `w3up-client` and `access`
Peeja pushed a commit to storacha/upload-service that referenced this issue Jan 29, 2025
🤖 I have created a release *beep* *boop*
---


##
[2.1.0](storacha/w3ui@core-v2.0.2...core-v2.1.0)
(2024-02-01)


### Features

* upgrade `core` dependencies for `plan/set`
([storacha#623](storacha/w3ui#623))
([b48a864](storacha/w3ui@b48a864))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants