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

fix glif token not being sent #69

Merged
merged 1 commit into from
May 15, 2024
Merged

fix glif token not being sent #69

merged 1 commit into from
May 15, 2024

Conversation

juliangruber
Copy link
Member

Would be nice to afterwards find out whether this is how Request works, or maybe a Zinnia bug.

@juliangruber juliangruber merged commit 3e75b81 into main May 15, 2024
1 check passed
@juliangruber juliangruber deleted the fix/missing-glif-token branch May 15, 2024 12:27
@juliangruber
Copy link
Member Author

@bajtos hindsight please

@juliangruber
Copy link
Member Author

@@ -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='
Copy link
Member

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='

Copy link
Member Author

Choose a reason for hiding this comment

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

#70

Comment on lines -5 to -9
export const RPC_REQUEST = new Request('https://api.node.glif.io/', {
headers: {
authorization: 'Bearer Bz8B9c8hKIp+/IyePpgORexkDDq+8c9atapgGuudtQ0='
}
})
Copy link
Member

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:
(...)

  • headers
    Any headers you want to add to your request, contained within a Headers object or an object literal with String values.

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/', {
Copy link
Member

@bajtos bajtos May 15, 2024

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:

https://github.com/filecoin-station/spark-api/blob/eea540e2086ab142b5bf9a94930fda1ec6d22032/lib/ie-contract.js#L5C8-L5C15

Copy link
Member Author

Choose a reason for hiding this comment

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

#70

@juliangruber juliangruber mentioned this pull request May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants