Skip to content

Conversation

@berezovskyi
Copy link
Member

@berezovskyi berezovskyi commented Jul 12, 2025

  • make the initial request using credentials: "include"
    • the code relying on the deprecated request lib was replaced with web-standard fetch()
  • non-localhost URIs get upgraded to HTTPS
  • CORS failing requests get retried with a CORS proxy without credentials
  • requests URIs ending with .ttl are parsed as Turtle not RDF/XML
  • add OSLC-deployed Umami script

Signed-off-by: Andrew Berezovskyi <andriib@kth.se>
- non-localhost URIs get upgraded to HTTPS
- CORS failing requests get retried with a CORS proxy without credentials
- requests URIs ending with .ttl are parsed as Turtle not RDF/XML

Signed-off-by: Andrew Berezovskyi <andriib@kth.se>
@berezovskyi berezovskyi requested review from Copilot and jamsden July 12, 2025 18:44
@berezovskyi berezovskyi changed the title Fix CORS requests Fix authenticated CORS requests Jul 12, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR replaces the deprecated request-based OSLC client with a modern fetch-based implementation that includes credentials, upgrades non-localhost HTTP to HTTPS, retries CORS failures via a proxy, and correctly parses Turtle resources.

  • Swap out OSLCServer in ResourceNavigator for the new OSLCClient
  • Implement fetch-based authGet with credential inclusion, HTTPS upgrade, and CORS proxy fallback
  • Distinguish .ttl URIs for Turtle parsing and choose Compact vs Resource based on content-type

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/ResourceNavigator.js Updated import from OSLCServer to new OSLCClient and adjusted instantiation
src/OSLCClient.js Added fetch-based client with default headers, auth flows, CORS retry, and parsing logic

callback(null, mockResponse, body);
} catch (corsError) {
console.error('CORS proxy request also failed:', corsError);
// throw corsError;
Copy link

Copilot AI Jul 12, 2025

Choose a reason for hiding this comment

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

In the CORS proxy fallback catch block, no callback is invoked, which can leave the caller hanging. Add a callback(corsError, null, null); (or similar) after logging to ensure errors propagate correctly.

Suggested change
// throw corsError;
callback(corsError, null, null);

Copilot uses AI. Check for mistakes.
// many 502s
// const corsProxyUrl = `https://everyorigin.jwvbremen.nl/get?url=${uri}`;
// works sometimes but seems to fail on fetching large TTLs
const corsProxyUrl = `https://cors-anywhere.com/${uri}`;
Copy link

Copilot AI Jul 12, 2025

Choose a reason for hiding this comment

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

The original URI is interpolated directly into the proxy URL, which may break on special characters or paths. Wrap uri with encodeURIComponent or use a query parameter to ensure the proxy request is formed correctly.

Suggested change
const corsProxyUrl = `https://cors-anywhere.com/${uri}`;
const corsProxyUrl = `https://cors-anywhere.com/${encodeURIComponent(uri)}`;

Copilot uses AI. Check for mistakes.
Comment on lines +32 to +47
/**
* Get cookie value from browser
*/
getCookie(key) {
if (typeof document !== 'undefined') {
const cookies = document.cookie.split(';');
for (let cookie of cookies) {
const [cookieName, cookieValue] = cookie.split('=').map(s => s.trim());
if (cookieName === key) {
return cookieValue;
}
}
}
return null;
}

Copy link

Copilot AI Jul 12, 2025

Choose a reason for hiding this comment

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

[nitpick] The getCookie method is defined but never used in this class. Consider removing it or integrating it into your authentication flow to keep the codebase lean.

Suggested change
/**
* Get cookie value from browser
*/
getCookie(key) {
if (typeof document !== 'undefined') {
const cookies = document.cookie.split(';');
for (let cookie of cookies) {
const [cookieName, cookieValue] = cookie.split('=').map(s => s.trim());
if (cookieName === key) {
return cookieValue;
}
}
}
return null;
}

Copilot uses AI. Check for mistakes.
Signed-off-by: Andrew Berezovskyi <andriib@kth.se>
@berezovskyi berezovskyi merged commit 2ab749f into main Jul 12, 2025
5 checks passed
@berezovskyi berezovskyi deleted the b-modern-fetch branch July 12, 2025 19:53
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.

2 participants