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

refactor: Stop reporting to Sentry Facebook multi-webhook error (no-changelog) #7743

Merged
merged 1 commit into from
Nov 17, 2023

Conversation

ivov
Copy link
Contributor

@ivov ivov commented Nov 17, 2023

@@ -195,6 +195,7 @@ export class FacebookTrigger implements INodeType {
throw new NodeOperationError(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we should create a NodeOperationWarning (and maybe NodeApiWarning), as people could forget to add severity, even when they know the severity.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was also thinking of a more general solution because this is neverending :)

Maybe we could have a separate nodes project in Sentry, and flip it so we only report errors there intentionally, e.g. ReportableNodeOperationError and ReportableNodeApiError - this is assuming the majority of errors in nodes-base are likely to be non-reportable because they come from external APIs, but I could be wrong to assume this.

Something to discuss with nodes team.

@n8n-assistant n8n-assistant bot added n8n team Authored by the n8n team node/improvement New feature or request labels Nov 17, 2023
Copy link

cypress bot commented Nov 17, 2023

4 flaky tests on run #2935 ↗︎

0 276 5 0 Flakiness 4

Details:

🌳 🖥️ browsers:node18.12.0-chrome107 🤖 ivov 🗃️ e2e/*
Project: n8n Commit: 14e6ba06aa
Status: Passed Duration: 06:02 💡
Started: Nov 17, 2023 9:34 AM Ended: Nov 17, 2023 9:40 AM
Flakiness  6-code-node.cy.ts • 2 flaky tests

View Output Video

Test Artifacts
Code node > Code editor > should execute the placeholder successfully in both modes Screenshots Video
... > generate code button should have correct state & tooltips Screenshots Video
Flakiness  28-resource-mapper.cy.ts • 2 flaky tests

View Output Video

Test Artifacts
Resource Mapper > should correctly delete single field Screenshots Video
Resource Mapper > should correctly delete all fields Screenshots Video

Review all test suite changes for PR #7743 ↗︎

Copy link
Contributor

✅ All Cypress E2E specs passed

@ivov ivov merged commit 05ed86c into master Nov 17, 2023
18 checks passed
@ivov ivov deleted the sentry-4640072705 branch November 17, 2023 10:39
MiloradFilipovic added a commit that referenced this pull request Nov 20, 2023
* master: (27 commits)
  fix: Include cypress TypeScript types in /cypress folder (no-changelog) (#7746)
  refactor(core): Stop reporting to Sentry `NodeApiError` outside 500 range (no-changelog) (#7753)
  fix(core): Guard against node not found on cancelling test webhook (#7750)
  fix(JotForm Trigger Node): Fix iteration on form loader (#7751)
  refactor(core): Stop reporting to Sentry unknown cred on mapping (no-changelog) (#7752)
  feat(core): Coordinate manual workflow activation and deactivation in multi-main scenario (#7643)
  ci: Fix "Release: Create Pull Request" workflow
  fix(editor): Fix Admin panel icon in the sidebar for cloud deployments (no-changelog) (#7738)
  fix(editor): Remove `n8nHooksNext` flag (no-changelog) (#7733)
  fix(editor): Show v1 banner dismiss button if owner (#7722)
  fix(GitHub Node): Fix issue preventing file edits on branches (#7734)
  fix(core): Fix all dependency versions for backend packages (no-changelog) (#7745)
  refactor(core): Convert dynamic node-parameter routes to a decorated controller (no-changelog) (#7284)
  refactor: Stop reporting to Sentry Facebook multi-webhook error (no-changelog) (#7743)
  refactor(core): Stop reporting to Sentry unrecognized node errors (no-changelog) (#7728)
  fix(core): Account for non-ASCII chars in filename on binary data download (#7742)
  ci: Fix DB tests and Workflow tests (no-changelog) (#7741)
  refactor: Extract Invitation routes to InvitationController (no-changelog) (#7726)
  fix(editor): Handle permission edge cases (empty scopes) (#7723)
  ci: Skip the regularly failing tests in 2-credentials.cy.ts (no-changelog) (#7736)
  ...
@janober
Copy link
Member

janober commented Nov 22, 2023

Got released with n8n@1.18.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
n8n team Authored by the n8n team node/improvement New feature or request Released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants