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

More fixes to proxy #1237

Merged
merged 2 commits into from
Dec 12, 2024
Merged

More fixes to proxy #1237

merged 2 commits into from
Dec 12, 2024

Conversation

aaronvg
Copy link
Contributor

@aaronvg aaronvg commented Dec 12, 2024

Important

Improved proxy handling in Rust and TypeScript by refining error messages, URL construction, and request routing logic.

  • Rust Proxy Handling:
    • Improved error message in to_base64_with_inferred_mime_type() in mod.rs to include media_url.url.
    • Refactored URL construction in fetch_with_proxy() in mod.rs to use new_proxy_url variable.
  • TypeScript Proxy Middleware:
    • Added logic in extension.ts to handle image GET requests by setting path to empty string.
    • Removed trailing slashes from paths in extension.ts.
    • Ensured baml-original-url header is used to route requests in extension.ts.
  • Miscellaneous:
    • Disabled ESLint rule @typescript-eslint/no-misused-promises in extension.ts.

This description was created by Ellipsis for e38f259. It will automatically update as commits are pushed.

Copy link

vercel bot commented Dec 12, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
baml ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 12, 2024 10:49pm

@aaronvg aaronvg enabled auto-merge December 12, 2024 22:39
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to d72586f in 23 seconds

More details
  • Looked at 142 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. typescript/vscode-ext/packages/vscode/src/extension.ts:228
  • Draft comment:
    Remove commented-out code to keep the codebase clean and maintainable.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code contains commented-out sections that are not needed. It's better to remove them to keep the code clean and maintainable.
2. typescript/vscode-ext/packages/vscode/src/extension.ts:251
  • Draft comment:
    Remove commented-out code to keep the codebase clean and maintainable.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code contains commented-out sections that are not needed. It's better to remove them to keep the code clean and maintainable.
3. typescript/vscode-ext/packages/vscode/src/extension.ts:253
  • Draft comment:
    Remove commented-out code to keep the codebase clean and maintainable.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code contains commented-out sections that are not needed. It's better to remove them to keep the code clean and maintainable.

Workflow ID: wflow_6eIxmxykr2AZxnfi


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on e38f259 in 13 seconds

More details
  • Looked at 25 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. typescript/vscode-ext/packages/vscode/src/extension.ts:197
  • Draft comment:
    Consider using a more sophisticated logging mechanism or removing console logs in production to avoid cluttering the console.
  • Reason this comment was not posted:
    Confidence changes required: 33%
    The console log statements in the proxy middleware might be useful for debugging but could clutter the console in production. Consider using a more sophisticated logging mechanism or removing them in production builds.
2. typescript/vscode-ext/packages/vscode/src/extension.ts:253
  • Draft comment:
    Consider using a more sophisticated logging mechanism or removing console logs in production to avoid cluttering the console.
  • Reason this comment was not posted:
    Confidence changes required: 33%
    The console log statements in the proxy middleware might be useful for debugging but could clutter the console in production. Consider using a more sophisticated logging mechanism or removing them in production builds.

Workflow ID: wflow_XPT3loUBIyRxh2YQ


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@aaronvg aaronvg added this pull request to the merge queue Dec 12, 2024
Merged via the queue into canary with commit 16054f5 Dec 12, 2024
10 of 11 checks passed
@aaronvg aaronvg deleted the proxy-gpt4ofix branch December 12, 2024 22:47
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