-
Notifications
You must be signed in to change notification settings - Fork 3
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
fix glif token not being sent #69
Conversation
@bajtos hindsight please |
@@ -23,7 +23,8 @@ async function rpc (method, ...params) { | |||
method: 'POST', | |||
headers: { | |||
'content-type': 'application/json', | |||
accepts: 'application/json' | |||
accepts: 'application/json', | |||
authorization: 'Bearer Bz8B9c8hKIp+/IyePpgORexkDDq+8c9atapgGuudtQ0=' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not a fan of having this configuration deep in the source code. Can we move the value to a constant exported by lib/constants.js
?
For example:
export const RPC_AUTH = 'Bearer Bz8B9c8hKIp+/IyePpgORexkDDq+8c9atapgGuudtQ0='
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export const RPC_REQUEST = new Request('https://api.node.glif.io/', { | ||
headers: { | ||
authorization: 'Bearer Bz8B9c8hKIp+/IyePpgORexkDDq+8c9atapgGuudtQ0=' | ||
} | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @juliangruber, for testing this. I should have tested it myself to verify my assumptions. 🙈
My understanding was based on the following docs:
https://developer.mozilla.org/en-US/docs/Web/API/Request/Request
options Optional
An object containing any custom settings that you want to apply to the request. The possible options are:
(...)
I did a bit of testing in the Web Inspector console, and at least in Safari, the constructor call new Request(RPC_REQUEST, { headers: { /* more headers */ } })
does not merge the headers from RPC_REQUEST with the headers from the options. Instead, the headers from the options replace those from the original request.
@@ -2,8 +2,4 @@ export const SPARK_VERSION = '1.11.2' | |||
export const MAX_CAR_SIZE = 200 * 1024 * 1024 // 200 MB | |||
export const APPROX_ROUND_LENGTH_IN_MS = 20 * 60_000 // 20 minutes | |||
|
|||
export const RPC_REQUEST = new Request('https://api.node.glif.io/', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be RPC_URL
now that it's just a URL string and not a request object.
We already use that same name in other places, e.g. here:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to afterwards find out whether this is how
Request
works, or maybe a Zinnia bug.