-
Notifications
You must be signed in to change notification settings - Fork 123
Investigate and test github issue 673 #674
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: aaron.d <aaron.d@nylas.com>
Co-authored-by: aaron.d <aaron.d@nylas.com>
|
Cursor Agent can help with this pull request. Just |
nylas + PlayerZeroView more in PlayerZero |
Remove redundant URL encoding tests and update description. Co-authored-by: aaron.d <aaron.d@nylas.com>
Co-authored-by: aaron.d <aaron.d@nylas.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #674 +/- ##
=======================================
Coverage 96.42% 96.42%
=======================================
Files 37 37
Lines 812 812
Branches 72 72
=======================================
Hits 783 783
Misses 26 26
Partials 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@AaronDDM, thanks for working on this! Re:
I'm trying to pinpoint why I would be seeing this error in my specific environment. FWIW, I'm running into this inside a Hono endpoint, running via the Cloudflare Vite plugin inside a developer simulated Workers environment. I did need to update to the latest plugin and wrangler version in order to pick up the requisite Node-compatible APIs. I'm not yet sure if there's something specific about that environment that would be causing URL composition to work differently. |
|
Hey,
This is helpful. I’ve actually been sick today and tomorrow is a holiday
here in Canada, so I’ll pick this up on Wednesday.
Given that you are using this on cloudflare workers, I suspect it has
something to do with its nodejs compatibility - but this is just a wild
guess. Looking at the code, we use the URL library to construct the final
request urls.
Thank you!
…On Mon, Sep 29, 2025 at 4:37 PM Lee Benson ***@***.***> wrote:
*leebenson* left a comment (nylas/nylas-nodejs#674)
<#674 (comment)>
@AaronDDM <https://github.com/AaronDDM>, thanks for working on this!
Re:
The investigation concluded that the issue is not a legitimate bug in the
current SDK, as the setQueryStrings method correctly handles URL encoding.
I'm trying to pinpoint why I would be seeing this error in my specific
environment.
FWIW, I'm running into this inside a Hono <https://hono.dev/> endpoint,
running via the Cloudflare Vite
<https://developers.cloudflare.com/workers/vite-plugin/> plugin inside a
developer simulated Workers environment.
I *did* need to update to the latest plugin and wrangler version in order
to pick up the requisite Node-compatible APIs. I'm not yet sure if there's
something specific about that environment that would be causing URL
composition to work differently.
—
Reply to this email directly, view it on GitHub
<#674 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACMWKB6XKRM4YITXCDPO5L3VGKCDAVCNFSM6AAAAACHXFRGEKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTGNBYHE3DGNJTG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Thank you! (and hey, I'm in Canada too!)
Hope you feel better soon.
On Mon, Sep 29, 2025 at 4:51 PM Aaron de Mello ***@***.***>
wrote:
… *AaronDDM* left a comment (nylas/nylas-nodejs#674)
<#674 (comment)>
Hey,
This is helpful. I’ve actually been sick today and tomorrow is a holiday
here in Canada, so I’ll pick this up on Wednesday.
Given that you are using this on cloudflare workers, I suspect it has
something to do with its nodejs compatibility - but this is just a wild
guess. Looking at the code, we use the URL library to construct the final
request urls.
Thank you!
On Mon, Sep 29, 2025 at 4:37 PM Lee Benson ***@***.***> wrote:
> *leebenson* left a comment (nylas/nylas-nodejs#674)
> <#674 (comment)>
>
> @AaronDDM <https://github.com/AaronDDM>, thanks for working on this!
>
> Re:
>
> The investigation concluded that the issue is not a legitimate bug in
the
> current SDK, as the setQueryStrings method correctly handles URL
encoding.
>
> I'm trying to pinpoint why I would be seeing this error in my specific
> environment.
>
> FWIW, I'm running into this inside a Hono <https://hono.dev/> endpoint,
> running via the Cloudflare Vite
> <https://developers.cloudflare.com/workers/vite-plugin/> plugin inside
a
> developer simulated Workers environment.
>
> I *did* need to update to the latest plugin and wrangler version in
order
> to pick up the requisite Node-compatible APIs. I'm not yet sure if
there's
> something specific about that environment that would be causing URL
> composition to work differently.
>
> —
> Reply to this email directly, view it on GitHub
> <#674 (comment)>,
> or unsubscribe
> <
https://github.com/notifications/unsubscribe-auth/AACMWKB6XKRM4YITXCDPO5L3VGKCDAVCNFSM6AAAAACHXFRGEKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTGNBYHE3DGNJTG4>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
—
Reply to this email directly, view it on GitHub
<#674 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFPW2XW44RWDP65UFVF4D33VGLT7AVCNFSM6AAAAACHXFRGEKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTGNBZGAYDSNBXGA>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Investigated GitHub issue #673 regarding URL encoding. The investigation concluded that the issue is not a legitimate bug in the current SDK, as the
setQueryStringsmethod correctly handles URL encoding.This PR adds a comprehensive suite of tests to
events.spec.tsto validate URL construction and encoding. These tests cover basic encoding, special characters, array parameters, metadata pairs, and ensure no double-encoding occurs, thus preventing future regressions related to URL query parameter handling.License
I confirm that this contribution is made under the terms of the MIT license and that I have the authority necessary to make this contribution on behalf of its copyright owner.