Skip to content

feat(skills-next): add cloudflare SDK references#238

Open
evanpurkhiser wants to merge 1 commit into
mainfrom
evanpurkhiser/feat-skills-next-add-cloudflare-sdk-references
Open

feat(skills-next): add cloudflare SDK references#238
evanpurkhiser wants to merge 1 commit into
mainfrom
evanpurkhiser/feat-skills-next-add-cloudflare-sdk-references

Conversation

@evanpurkhiser

Copy link
Copy Markdown
Member

Direct LLM port of the existing cloudflare SDK skill into the skills-next per-SDK reference layout under skills-next/references/sdks/cloudflare/. This content has NOT been reviewed at all — it is a machine-generated port of the existing SDK skill, and every file should be treated as unverified.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a645ba7. Configure here.

const url = samplingContext.attributes?.["url.full"] as string | undefined;

// Always trace health checks
if (url?.includes("/health")) return 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Health check sampler drops traces

Medium Severity

The tracesSampler example comments that health checks should always be traced, but returns 0 for URLs containing /health. In Sentry, 0 means never sample, so health routes would be excluded from tracing—the opposite of the stated intent.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit a645ba7. Configure here.

Comment on lines +373 to +377
| Error Monitoring | `references/error-monitoring.md` | Always (baseline) — unhandled exceptions, manual capture, scopes, enrichment |
| Tracing | `references/tracing.md` | HTTP request tracing, outbound fetch spans, D1 query spans, distributed tracing |
| Logging | `references/logging.md` | Structured logs via `Sentry.logger.*`, log-to-trace correlation |
| Crons | `references/crons.md` | Scheduled handler monitoring, `withMonitor`, check-in API |
| Durable Objects | `references/durable-objects.md` | Instrument Durable Object classes for error capture and spans |

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The relative links in index.md are incorrect. They use a references/ prefix, but the linked files are siblings in the same directory, breaking path resolution.
Severity: HIGH

Suggested Fix

Remove the references/ prefix from the file paths on lines 373-377. For example, change references/error-monitoring.md to error-monitoring.md. This will make the paths relative to the current file's directory, allowing them to resolve correctly.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: skills-next/references/sdks/cloudflare/index.md#L373-L377

Potential issue: In the `skills-next/references/sdks/cloudflare/index.md` file, the
relative paths used to link to other markdown files are incorrect. The file attempts to
link to sibling files like `error-monitoring.md` using paths prefixed with `references/`
(e.g., `references/error-monitoring.md`). Because the `index.md` file and the target
files are in the same directory, this creates an invalid path that resolves to a
non-existent location (`.../cloudflare/references/error-monitoring.md`). This will
prevent an LLM agent from locating and processing these reference files, breaking the
skill's functionality.

Did we get this right? 👍 / 👎 to inform future reviews.

Comment on lines +72 to +73
// Always trace health checks
if (url?.includes("/health")) return 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The code comment // Always trace health checks is incorrect. The following code, return 0;, actually excludes health checks from being traced, contradicting the comment's description.
Severity: MEDIUM

Suggested Fix

Update the comment to accurately reflect the code's behavior. Change it to // Exclude health checks from tracing or a similar description. Alternatively, if the intent is to always trace them, change the code to return 1;.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: skills-next/references/sdks/cloudflare/tracing.md#L72-L73

Potential issue: A code comment in an example for `tracesSampler` is factually incorrect
and misleading. The comment `// Always trace health checks` is immediately followed by
the code `return 0;`. According to Sentry documentation, returning `0` from the sampler
function explicitly prevents the transaction from being sampled and sent, effectively
excluding it from tracing. This contradiction will cause developers who copy this
example to believe they are tracing health checks when they are actually dropping them,
leading to confusion and debugging time when the expected traces do not appear in
Sentry.

Did we get this right? 👍 / 👎 to inform future reviews.

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.

1 participant