optimization: When using custom SSE request,Authorization header can still be automatically attached to the SSE request.#478
Conversation
…header can still be automatically attached to the SSE request.
ochafik
left a comment
There was a problem hiding this comment.
Hi @chenxi-null, thanks for sending this out! I took the liberty to refactor / simplify the code a bit, let me know your thoughts :-)
.gitignore
Outdated
|
|
||
| # Stores VSCode versions used for testing VSCode extensions | ||
| .vscode-test | ||
| .vscode/ |
There was a problem hiding this comment.
Not sure this belongs here?
There was a problem hiding this comment.
I want to make the .vscode folder untracked by Git.
ochafik
left a comment
There was a problem hiding this comment.
There's now some conflicts coming in from main onto the tests, could you have a look?
@ochafik Okay, I'll resolve the conflicts. |
…sh-token # Conflicts: # src/client/sse.test.ts # src/client/sse.ts
|
@ochafik I have resolved the code conflicts. By the way, I find a test case failure which is already existed in main branch. So I can't pass all test cases, but it's caused by the other code in main branch. It need someone else to fix it.
|
|
@ochafik I've refactored and modified the code. Please have a look. |
ae99c1b to
b08ad9f
Compare
…connection-logging Normalize connection logging
0845a57 to
c94ba4b
Compare
When using the custom SSE request, the
Authorizationheader can still be automatically attached to the SSE request.Motivation and Context
Current:
When user customizes the initial SSE request, it will prevent an
Authorizationheader from being automatically attached to the SSE request. It require user to fetch tokens fromauthProviderand set theAuthorizationheader manually.Ref:
typescript-sdk/src/client/sse.ts
Lines 36 to 44 in bced33d
Goal:
Simplify the code and reduce the risk of misuse.
There is an misused case: CherryHQ/cherry-studio#5709
How Has This Been Tested?
npm test -- src/client/sse.test.ts -t "refreshes expired token during SSE connection"Breaking Changes
Types of changes
Checklist
Additional context