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

[research] API Publish with Intermittent script timeout #499

Open
vasco-santos opened this issue Sep 28, 2021 · 3 comments · Fixed by #505
Open

[research] API Publish with Intermittent script timeout #499

vasco-santos opened this issue Sep 28, 2021 · 3 comments · Fixed by #505
Labels
kind/bug A bug in existing code (including security flaws) P4 stack/quality-velocity topic/devexp Developer experience improvements, onboard efficiencies, time savers.

Comments

@vasco-santos
Copy link
Contributor

Sometimes when publishing to CF workers, or starting a CF Worker in Dev mode, the script timeouts after the build finishes.

It is likely related to the S3 client dependency added to the bundle in #417

I could not replicate this locally, but @olizilla can. On latest releases, it happened once and it might become annoying if it is recurrent. Let's track this and if it is frequent, we should probably just rely on S3 HTTP PutObject API instead, given all the other most used s3 client are not well suited for a browser environment.

@vasco-santos vasco-santos added kind/bug A bug in existing code (including security flaws) need/triage Needs initial labeling and prioritization labels Sep 28, 2021
olizilla added a commit that referenced this issue Sep 30, 2021
- delete the source map after it has been sent to sentry but before it would be uploaded to cloudflare
- skip building a sourece map in dev
- skip the extracting comments to a license file

builds on #505
may help with #499

License: (Apache-2.0 AND MIT)
Signed-off-by: Oli Evans <oli@tableflip.io>
olizilla added a commit that referenced this issue Sep 30, 2021
Match up the release name reported during deploy with the release name reported at run time. This PR lines things up so releases appear in sentry like `web3-api@<semver>` which it automagically parses nicely. We currently we deploy as `<sort git sha>` but we report as `api-<semver>`.

see: https://sentry.io/organizations/protocol-labs-it/releases/web3-api%403.5.1/?project=5866408

- `SENTRY_RELEASE` env var is set during the build, and used for both creating the release at build time and on error reports so they get matched up and our sourcemap is used to unravel the stacktrace.
- Rename the bundle to be `worker.js` as that is what cloudflare renames (!?) it to on deploy, and errors are reported as coming from `worker.js` in production, so that has to match or else the source map is not applied.
- remove the `//# sourceMappingURL=...` link from the end of the js bundle by switching to `devtool: hidden-source-map` in the webpack config. I have no idea whey this is necessary. see: robertcepa/toucan-js#54 (comment)
- add commit info to releases in a way that sentry understands, and can match up to our repo with `setCommits: { auto: true }` see: https://github.com/getsentry/sentry-webpack-plugin#optionssetcommits

This fixes our double release counting in sentry and make source maps work for _much error readability_!

<img width="848" alt="Screenshot 2021-09-29 at 16 24 53" src="https://user-images.githubusercontent.com/58871/135340773-7f321748-4aae-401e-9dc9-c4ae5d19a696.png">

I have also connected up the repo in Sentry so we can see the commits that go in to the release.

<img width="364" alt="Screenshot 2021-09-29 at 21 09 37" src="https://user-images.githubusercontent.com/58871/135341267-d67026d7-881f-4f7b-a314-fbc102c75d37.png">


### Follow on work

- We can spare ourselves from cf worker upload errors by just not uploading our 4MB source map as part of deployment. Use the webpack remove files plugin to delete the source map after it has been uploaded to sentry, as we're just not using in cloudflare! would fix #499 see: #506 
- Add COMMITSHA and BRANCH env vars, and a `/version` endpoint to report it as nft.storage does. https://github.com/ipfs-shipyard/nft.storage/blob/7102ad5fc06d90a85027e40fa0aece3429b833e7/packages/api/src/index.js#L58-L64

License: (Apache-2.0 AND MIT)
Signed-off-by: Oli Evans <oli@tableflip.io>
olizilla added a commit that referenced this issue Sep 30, 2021
- delete the source map after it has been sent to sentry but before it would be uploaded to cloudflare
- skip building a sourece map in dev
- skip the extracting comments to a license file

builds on #505
may help with #499

License: (Apache-2.0 AND MIT)
Signed-off-by: Oli Evans <oli@tableflip.io>
@olizilla olizilla reopened this Sep 30, 2021
@atopal atopal added P1 High: Likely tackled by core team if no one steps up and removed need/triage Needs initial labeling and prioritization labels Oct 1, 2021
@vasco-santos
Copy link
Contributor Author

cc @hugomrdias

@hugomrdias hugomrdias self-assigned this Oct 26, 2021
@atopal atopal changed the title API Publish with Intermittent script timeout [research] API Publish with Intermittent script timeout Nov 24, 2021
@dchoi27
Copy link
Contributor

dchoi27 commented Jan 11, 2022

hey @vasco-santos are we still experiencing this?

@dchoi27 dchoi27 added P2 Medium: Good to have, but can wait until someone steps up and removed P1 High: Likely tackled by core team if no one steps up labels Jan 11, 2022
@vasco-santos
Copy link
Contributor Author

Way less often since #679 but still get it a few times (only in staging I believe, so maybe the production app has different capabilities per the CF plan)

@dchoi27 dchoi27 added P4 topic/devexp Developer experience improvements, onboard efficiencies, time savers. and removed P2 Medium: Good to have, but can wait until someone steps up labels Jan 11, 2022
@hugomrdias hugomrdias removed their assignment Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in existing code (including security flaws) P4 stack/quality-velocity topic/devexp Developer experience improvements, onboard efficiencies, time savers.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants