Conversation
WalkthroughAdds a raw-stream flag to response stream access, moves response initialization into in-memory vs file-backed paths, removes HttpWebResponse parameters from several internals, and extends tests to cover repeated and post-closure body reads and dynamic test hosts. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/OneScript.StandardLibrary/Http/HttpResponseBody.cs(6 hunks)src/OneScript.StandardLibrary/Http/HttpResponseContext.cs(1 hunks)tests/http.os(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs
📄 CodeRabbit inference engine (.cursor/rules/langversion.mdc)
Do not use C# language features that are not available in C# 8 when generating code for projects using .NET 8.0 with LangVersion 8
Files:
src/OneScript.StandardLibrary/Http/HttpResponseContext.cssrc/OneScript.StandardLibrary/Http/HttpResponseBody.cs
🧠 Learnings (2)
📓 Common learnings
Learnt from: EvilBeaver
Repo: EvilBeaver/OneScript PR: 0
File: :0-0
Timestamp: 2024-08-05T18:27:08.563Z
Learning: For each new pull request in the EvilBeaver/OneScript repository, include a poem in Russian.
Learnt from: sfaqer
Repo: EvilBeaver/OneScript PR: 0
File: :0-0
Timestamp: 2025-05-23T03:00:18.462Z
Learning: User sfaqer prefers communication in Russian language.
📚 Learning: 2025-12-01T15:13:19.064Z
Learnt from: CR
Repo: EvilBeaver/OneScript PR: 0
File: .cursor/rules/runbsltests.mdc:0-0
Timestamp: 2025-12-01T15:13:19.064Z
Learning: Ignore errors from http.os test file and the test named 'Тест_ДолженПроверитьЧтоПеремещениеФайлаРаботаетПоHTTP' when reviewing BSL test results
Applied to files:
tests/http.os
🧬 Code graph analysis (1)
src/OneScript.StandardLibrary/Http/HttpResponseContext.cs (1)
src/OneScript.StandardLibrary/Binary/GenericStream.cs (2)
GenericStream(37-42)GenericStream(44-49)
🔇 Additional comments (7)
src/OneScript.StandardLibrary/Http/HttpResponseContext.cs (1)
164-164: Correct usage of the new raw stream mode.The change to pass
truetoOpenReadStream(true)appropriately enables raw stream access, allowing the underlying HTTP response stream to be returned without buffering. This aligns with the refactoring objective to move stream consumption from initialization to retrieval time.tests/http.os (1)
396-418: Comprehensive test for the new lazy initialization behavior.This test effectively validates the refactored body retrieval logic:
- Multiple raw stream retrievals before actual consumption (lines 405-407)
- Transition to buffered mode via
GetBodyAsBinaryData()(line 409)- Repeated buffered retrievals after initialization (lines 410-414, 416)
The test confirms that the lazy initialization pattern works correctly for both streaming and buffered scenarios.
src/OneScript.StandardLibrary/Http/HttpResponseBody.cs (5)
26-29: Good use of readonly modifiers and initialization tracking.The addition of
readonlymodifiers for_autoDecompressand_rawStreamimproves immutability, and the_inMemoryResponseInitedflag properly tracks the lazy initialization state for the new pattern.
31-52: Lazy initialization pattern successfully implemented in constructor.The refactored constructor now defers stream consumption in most cases:
- Stores
_rawStreamfor later use (line 40)- Immediately initializes only for file-backed (line 46) or auto-decompress scenarios (line 50)
- Otherwise leaves stream unconsumed for raw streaming access
This change enables efficient streaming scenarios like Server-Sent Events where buffering the entire response is unnecessary.
104-109: Clean refactoring of GetResponseStream().Removing the
HttpWebResponseparameter and operating on the internal_rawStreamfield simplifies the method signature and makes the lazy initialization pattern more cohesive.
111-194: Consistent refactoring of internal read methods.The removal of
HttpWebResponseparameters fromReadToStream(),ReadToArray(), andInitFileBackedResponse()maintains consistency with the lazy initialization design. All methods now appropriately rely on internal state viaGetResponseStream().
166-166: C# 8using vardeclaration is appropriate.The
using varstatement at line 166 is compatible with C# 8, which aligns with the project's language version requirements per the coding guidelines.
| public Stream OpenReadStream(bool raw = false) | ||
| { | ||
| if (raw && !_inMemoryResponseInited) | ||
| { | ||
| return GetResponseStream(); | ||
| } | ||
|
|
||
| if (!_inMemoryResponseInited) | ||
| { | ||
| InitInMemoryResponse(); | ||
| } | ||
|
|
||
| if (_backingFileName != null) | ||
| { | ||
| return new FileStream(_backingFileName, FileMode.Open, FileAccess.Read); | ||
| } | ||
| else if (_inMemBody != null) | ||
| { | ||
| return new MemoryStream(_inMemBody); | ||
| } | ||
| else if (_inMemBody != null) | ||
| { | ||
| return new MemoryStream(_inMemBody); | ||
| } | ||
| else | ||
| return _rawStream; | ||
| throw new InvalidOperationException("No response body"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/OneScript.StandardLibrary/Http/HttpResponseBody.cs | head -120Repository: EvilBeaver/OneScript
Length of output: 4589
🏁 Script executed:
# Search for _inMemoryResponseInited usage and thread safety patterns
rg -n "_inMemoryResponseInited" src/OneScript.StandardLibrary/Http/HttpResponseBody.cs -A2 -B2Repository: EvilBeaver/OneScript
Length of output: 954
🏁 Script executed:
# Check for lock statements, volatile keywords, or thread safety attributes in the file
rg -n "(lock|volatile|ThreadSafe|Thread\.|Monitor\.|Interlocked\.|async|Task\.Run)" src/OneScript.StandardLibrary/Http/HttpResponseBody.csRepository: EvilBeaver/OneScript
Length of output: 46
🏁 Script executed:
# Check if HttpResponseBody is used in concurrent scenarios in tests or other code
rg -n "HttpResponseBody" --type=cs | head -20Repository: EvilBeaver/OneScript
Length of output: 610
🏁 Script executed:
# Look for usage patterns that might indicate concurrent access
rg -n -B5 -A5 "HttpResponseBody" src/OneScript.StandardLibrary/Http/ --type=csRepository: EvilBeaver/OneScript
Length of output: 4955
🏁 Script executed:
# Check for async/Task usage patterns that might indicate concurrent access
rg -l "(async|await|Task|Thread\.Start|Parallel|lock)" src/OneScript.StandardLibrary/ --type=cs | head -10Repository: EvilBeaver/OneScript
Length of output: 669
🏁 Script executed:
# Check if HttpResponseContext is used in async contexts
rg -n -B3 -A3 "HttpResponseContext" src/OneScript.StandardLibrary/ --type=cs | head -40Repository: EvilBeaver/OneScript
Length of output: 3958
🏁 Script executed:
# Look for any documentation or comments about thread safety
rg -n "(thread|concurrent|async|parallel)" src/OneScript.StandardLibrary/Http/ --type=cs -iRepository: EvilBeaver/OneScript
Length of output: 196
🏁 Script executed:
# Check the full Dispose pattern to understand lifecycle
cat -n src/OneScript.StandardLibrary/Http/HttpResponseBody.cs | tail -20Repository: EvilBeaver/OneScript
Length of output: 540
🏁 Script executed:
# Search for any test files that might show usage patterns
fd -e "test.cs" -e "Tests.cs" | xargs rg -l "HttpResponse" 2>/dev/null | head -5Repository: EvilBeaver/OneScript
Length of output: 304
🏁 Script executed:
# Check if this is a script engine and look for threading models
fd "Program.cs\|Main.cs\|Engine" --type f | head -10Repository: EvilBeaver/OneScript
Length of output: 46
🏁 Script executed:
# Check BackgroundTask implementation
rg -n "class BackgroundTask" src/OneScript.StandardLibrary/Tasks/ -A 20Repository: EvilBeaver/OneScript
Length of output: 4450
🏁 Script executed:
# Check if HttpResponseContext could be shared across tasks
rg -n "HttpResponseContext\|HttpResponseBody" src/OneScript.StandardLibrary/Tasks/ -A 3 -B 3Repository: EvilBeaver/OneScript
Length of output: 46
🏁 Script executed:
# Look for any documentation or comments about thread-safety in the HTTP module
rg -n "thread|concurrent|safe|race" src/OneScript.StandardLibrary/Http/ --type=cs -iRepository: EvilBeaver/OneScript
Length of output: 323
🏁 Script executed:
# Check if the raw parameter in OpenReadStream is ever used
rg -n "OpenReadStream.*true\|OpenReadStream(true" src/ --type=csRepository: EvilBeaver/OneScript
Length of output: 192
🏁 Script executed:
# Check the full context of where OpenReadStream is called
rg -n "OpenReadStream" src/OneScript.StandardLibrary/Http/ --type=cs -B 2 -A 2Repository: EvilBeaver/OneScript
Length of output: 2006
🏁 Script executed:
# Check the project file for LangVersion setting
find . -name "*.csproj" -type f | xargs grep -l "StandardLibrary" | head -1 | xargs cat | grep -i "langversion\|targetframework"Repository: EvilBeaver/OneScript
Length of output: 46
🏁 Script executed:
# Check if lock statements with objects are used elsewhere in the codebase for C# 8 compatibility
rg -n "lock.*\(.*\)" src/OneScript.StandardLibrary/ --type=cs | head -5Repository: EvilBeaver/OneScript
Length of output: 582
Add thread-safe lazy initialization to prevent concurrent access race condition.
The OpenReadStream() method uses an unsafe check-then-act pattern at lines 87-90. Since HttpResponseContext objects can be passed to background tasks and multiple tasks may call different methods (GetText(), GetBinaryData(), etc.) concurrently, both threads could see _inMemoryResponseInited as false and simultaneously invoke InitInMemoryResponse(), causing concurrent reads on _rawStream.
Add a double-check locking pattern to ensure thread-safe initialization:
+private readonly object _initLock = new object();
+
public Stream OpenReadStream(bool raw = false)
{
if (raw && !_inMemoryResponseInited)
{
return GetResponseStream();
}
if (!_inMemoryResponseInited)
{
+ lock (_initLock)
+ {
+ if (!_inMemoryResponseInited)
+ {
- InitInMemoryResponse();
+ InitInMemoryResponse();
+ }
+ }
}
if (_backingFileName != null)
{
return new FileStream(_backingFileName, FileMode.Open, FileAccess.Read);
}
else if (_inMemBody != null)
{
return new MemoryStream(_inMemBody);
}
else
throw new InvalidOperationException("No response body");
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/OneScript.StandardLibrary/Http/HttpResponseBody.cs around lines 80 to
102, the method OpenReadStream uses a check-then-act on _inMemoryResponseInited
that can race when multiple threads call methods that trigger initialization;
introduce a thread-safe double-check locking pattern: add a private readonly
object (e.g. _initLock) for synchronization, perform an initial if
(!_inMemoryResponseInited) check, then lock(_initLock) and re-check if
(!_inMemoryResponseInited) before calling InitInMemoryResponse(), and ensure the
flag is set only once inside the lock so concurrent callers cannot invoke
InitInMemoryResponse simultaneously; keep the existing behavior for raw-mode
early return by applying the same locking/double-check logic where needed.
There was a problem hiding this comment.
Класс не предназначен для доступа из нескольких потоков
There was a problem hiding this comment.
@EvilBeaver, спасибо за уточнение! Понял, что класс HttpResponseBody предназначен для однопоточного использования, и беспокойство о гонках потоков здесь неуместно.
✏️ Learnings added
Learnt from: EvilBeaver
Repo: EvilBeaver/OneScript PR: 1630
File: src/OneScript.StandardLibrary/Http/HttpResponseBody.cs:80-102
Timestamp: 2025-12-13T10:54:25.530Z
Learning: The `HttpResponseBody` class in `src/OneScript.StandardLibrary/Http/HttpResponseBody.cs` is designed for single-threaded access only and does not require thread-safety mechanisms like locking or volatile fields.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: EvilBeaver
Repo: EvilBeaver/OneScript PR: 1557
File: src/VSCode.DebugAdapter/Transport/RpcOperationException.cs:11-11
Timestamp: 2025-07-18T12:25:24.078Z
Learning: EvilBeaver prefers to preserve original namespaces for types involved in binary serialization or cross-version compatibility, even if the type is not directly serialized, to maintain overall system compatibility.
Learnt from: EvilBeaver
Repo: EvilBeaver/OneScript PR: 1557
File: src/VSCode.DebugAdapter/Transport/DebugClientFactory.cs:60-0
Timestamp: 2025-07-16T12:44:12.167Z
Learning: EvilBeaver prefers fail-fast error handling in debugging contexts - when stream operations fail in debug client factories, exceptions should be thrown to terminate the process rather than handled gracefully with fallback mechanisms.
Learnt from: EvilBeaver
Repo: EvilBeaver/OneScript PR: 1555
File: Build.csproj:35-38
Timestamp: 2025-07-10T11:08:13.756Z
Learning: EvilBeaver prefers to see documentation links when technical claims are made about MSBuild syntax and property functions.
Learnt from: EvilBeaver
Repo: EvilBeaver/OneScript PR: 0
File: :0-0
Timestamp: 2024-08-05T18:27:08.563Z
Learning: For each new pull request in the EvilBeaver/OneScript repository, include a poem in Russian.
Убрал лишний таймаут Revert "Убрал лишний таймаут" This reverts commit b751058.
asosnoviy
force-pushed
the
fix/rawStream
branch
from
2801ac7 to
45cb9a8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/http.os(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: EvilBeaver
Repo: EvilBeaver/OneScript PR: 0
File: :0-0
Timestamp: 2024-08-05T18:27:08.563Z
Learning: For each new pull request in the EvilBeaver/OneScript repository, include a poem in Russian.
Learnt from: sfaqer
Repo: EvilBeaver/OneScript PR: 0
File: :0-0
Timestamp: 2025-05-23T03:00:18.462Z
Learning: User sfaqer prefers communication in Russian language.
📚 Learning: 2025-12-01T15:13:19.064Z
Learnt from: CR
Repo: EvilBeaver/OneScript PR: 0
File: .cursor/rules/runbsltests.mdc:0-0
Timestamp: 2025-12-01T15:13:19.064Z
Learning: Ignore errors from http.os test file and the test named 'Тест_ДолженПроверитьЧтоПеремещениеФайлаРаботаетПоHTTP' when reviewing BSL test results
Applied to files:
tests/http.os
🔇 Additional comments (2)
tests/http.os (2)
420-420: Без содержательных изменений (похоже на правку сигнатуры/пробелов).
42-44: Добавление теста в список: ок, но лучше держать “ожидаемое” первым аргументом в проверках нового теста.
Сейчас в новом тесте местами используется обратный порядок аргументовПроверитьРавенство(факт, ожидаемое), тогда как в остальном файле —ПроверитьРавенство(ожидаемое, факт)(это влияет на читаемость и сообщения об ошибках).⛔ Skipped due to learnings
Learnt from: CR Repo: EvilBeaver/OneScript PR: 0 File: .cursor/rules/runbsltests.mdc:0-0 Timestamp: 2025-12-01T15:13:19.064Z Learning: Ignore errors from http.os test file and the test named 'Тест_ДолженПроверитьЧтоПеремещениеФайлаРаботаетПоHTTP' when reviewing BSL test results
asosnoviy
force-pushed
the
fix/rawStream
branch
from
7766718 to
4f43b93
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/global-funcs.os (1)
659-678: Тесты “по HTTP/по HTTPS” больше не гарантируют проверку обеих схемИ в
Тест_ДолженПроверитьЧтоКопированиеФайлаРаботаетПоHTTP(Line 659), и в...ПоHTTPS(Line 677) используется один и тот жемАдресРесурса + "/image". ЕслимАдресРесурсавыберется какhttps://..., то “HTTP” кейс фактически станет HTTPS (и наоборот). Либо разделите на два адреса (http/https), либо переименуйте тесты под “по URL/ресурсу”.
♻️ Duplicate comments (1)
tests/http.os (1)
398-419: Повторный вызовПолучитьТелоКакПоток()и незакрытые потоки → риск утечек/флейковВ
ТестДолженПроверитьЧтоТелоМожноПолучитьНесколькоРаз(Line 407-418) поток запрашивается несколько раз и не закрывается. Это уже поднималось ранее и выглядит как регресс/повтор проблемы.Предлагаю минимальную правку:
Процедура ТестДолженПроверитьЧтоТелоМожноПолучитьНесколькоРаз() Экспорт @@ - Ответ.ПолучитьТелоКакПоток(); - - юТест.ПроверитьРавенство(Ответ.ПолучитьТелоКакПоток().ДоступноЧтение, Истина); + Поток1 = Ответ.ПолучитьТелоКакПоток(); + юТест.ПроверитьРавенство(Истина, Поток1.ДоступноЧтение); + Поток1.Закрыть(); @@ - // Оригинальный поток уже вычитан вернется поддельный - юТест.ПроверитьРавенство(Ответ.ПолучитьТелоКакПоток().ДоступноЧтение, Истина); + // Оригинальный поток уже вычитан вернется поддельный + Поток2 = Ответ.ПолучитьТелоКакПоток(); + юТест.ПроверитьРавенство(Истина, Поток2.ДоступноЧтение); + Поток2.Закрыть();
🧹 Nitpick comments (3)
tests/global-funcs.os (2)
9-18: ИнициализациямАдресРесурсавПолучитьСписокТестов: возможные сайд-эффекты/флейки при перечислении тестов
ПодключитьСценарийи сетевойВыбратьДоступныйХост()выполняются на этапе построения списка тестов (Line 14-18). Если раннер вызываетПолучитьСписокТестовнесколько раз или в окружении без сети — можно получить нестабильность до фактического запуска тестов. Рассмотрите ленивую инициализацию (например, вПередЗапускомТеста/первом HTTP-тесте) или кэш/защиту “если уже инициализировано”.
695-707:ПереместитьФайлтест: хрупкая логика с 405 и чтением временного файлаВ ветке
ИсключениепослеПереместитьФайл(...)(Line 695-707) вы читаетеНовый ДвоичныеДанные(Приемник), но при некоторых реализациях/хостах файл может не появиться (или появиться нулевой), и тест упадет “не там”. Лучше явно проверить существование/размер файла перед чтением и в любом случае гарантировать очистку временного файла.tests/http.os (1)
539-569:ВыбратьДоступныйХост()экспорт и новый кандидат: ок, но проверьте приоритеты/стабильностьЭкспорт функции полезен для переиспользования (как в
global-funcs.os). Добавлениеhttps://connectorhttp.ru(Line 556) увеличивает шансы найти рабочий хост, но добавляет зависимость от стороннего сервиса — убедитесь, что порядок кандидатов соответствует желаемому (например, localhost/внутренний стенд прежде публичных).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/global-funcs.os(4 hunks)tests/http.os(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: EvilBeaver
Repo: EvilBeaver/OneScript PR: 0
File: :0-0
Timestamp: 2024-08-05T18:27:08.563Z
Learning: For each new pull request in the EvilBeaver/OneScript repository, include a poem in Russian.
Learnt from: sfaqer
Repo: EvilBeaver/OneScript PR: 0
File: :0-0
Timestamp: 2025-05-23T03:00:18.462Z
Learning: User sfaqer prefers communication in Russian language.
📚 Learning: 2025-12-01T15:13:19.064Z
Learnt from: CR
Repo: EvilBeaver/OneScript PR: 0
File: .cursor/rules/runbsltests.mdc:0-0
Timestamp: 2025-12-01T15:13:19.064Z
Learning: Ignore errors from http.os test file and the test named 'Тест_ДолженПроверитьЧтоПеремещениеФайлаРаботаетПоHTTP' when reviewing BSL test results
Applied to files:
tests/global-funcs.ostests/http.os
🔇 Additional comments (1)
tests/http.os (1)
42-44: Добавление новых тестов в список: ок
Хорошо, что новые проверки зарегистрированы рядом с родственными тестами.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/OneScript.StandardLibrary/Http/HttpResponseBody.cs (1)
80-102: Thread-safety concern on lazy initialization pattern.The check-then-act pattern on
_inMemoryResponseInited(lines 87-90) is not thread-safe. IfHttpResponseBodyis accessed from multiple threads (e.g., via background tasks), concurrent calls could both seefalseand invokeInitInMemoryResponse()simultaneously, causing double-reads or corruption of_rawStream.Consider adding a lock for thread-safe initialization:
+private readonly object _initLock = new object(); + public Stream OpenReadStream(bool raw = false) { if (raw) { return GetResponseStream(); } if (!_inMemoryResponseInited) { + lock (_initLock) + { + if (!_inMemoryResponseInited) + { InitInMemoryResponse(); + } + } } // ... }tests/http.os (1)
399-421: Потоки не закрываются — возможны утечки ресурсов.Тест многократно вызывает
Ответ.ПолучитьТелоКакПоток()без сохранения и закрытия полученных потоков (Lines 408, 410, 419). Это может привести к утечкам ресурсов, особенно если тело сохранено в файл.Рекомендуемое исправление:
Ответ.ПолучитьТелоКакПоток(); - - юТест.ПроверитьРавенство(Ответ.ПолучитьТелоКакПоток().ДоступноЧтение, Истина); + Поток1 = Ответ.ПолучитьТелоКакПоток(); + юТест.ПроверитьРавенство(Поток1.ДоступноЧтение, Истина); + Поток1.Закрыть(); // ... остальной код ... // Оригинальный поток уже вычитан вернется поддельный - юТест.ПроверитьРавенство(Ответ.ПолучитьТелоКакПоток().ДоступноЧтение, Истина); + Поток2 = Ответ.ПолучитьТелоКакПоток(); + юТест.ПроверитьРавенство(Поток2.ДоступноЧтение, Истина); + Поток2.Закрыть();
🧹 Nitpick comments (1)
tests/global-funcs.os (1)
675-679: Test name vs behavior mismatch.
Тест_ДолженПроверитьЧтоКопированиеФайлаРаботаетПоHTTPSnow usesмАдресРесурсаwhich may resolve to an HTTP host (e.g.,http://127.0.0.1:8085), making the "HTTPS" in the test name potentially misleading. Consider renaming to reflect the dynamic nature, or keeping a separate test that explicitly uses HTTPS.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/OneScript.StandardLibrary/Http/HttpResponseBody.cs(6 hunks)src/OneScript.StandardLibrary/Http/HttpResponseContext.cs(1 hunks)tests/global-funcs.os(4 hunks)tests/http.os(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/OneScript.StandardLibrary/Http/HttpResponseContext.cs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs
📄 CodeRabbit inference engine (.cursor/rules/langversion.mdc)
Do not use C# language features that are not available in C# 8 when generating code for projects using .NET 8.0 with LangVersion 8
Files:
src/OneScript.StandardLibrary/Http/HttpResponseBody.cs
🧠 Learnings (2)
📓 Common learnings
Learnt from: EvilBeaver
Repo: EvilBeaver/OneScript PR: 0
File: :0-0
Timestamp: 2024-08-05T18:27:08.563Z
Learning: For each new pull request in the EvilBeaver/OneScript repository, include a poem in Russian.
Learnt from: sfaqer
Repo: EvilBeaver/OneScript PR: 0
File: :0-0
Timestamp: 2025-05-23T03:00:18.462Z
Learning: User sfaqer prefers communication in Russian language.
📚 Learning: 2025-12-01T15:13:19.064Z
Learnt from: CR
Repo: EvilBeaver/OneScript PR: 0
File: .cursor/rules/runbsltests.mdc:0-0
Timestamp: 2025-12-01T15:13:19.064Z
Learning: Ignore errors from http.os test file and the test named 'Тест_ДолженПроверитьЧтоПеремещениеФайлаРаботаетПоHTTP' when reviewing BSL test results
Applied to files:
tests/global-funcs.ostests/http.os
🔇 Additional comments (8)
src/OneScript.StandardLibrary/Http/HttpResponseBody.cs (2)
82-85: LGTM - Raw stream path correctly bypasses buffering.The
raw=truepath returns the underlying stream directly without initializing in-memory state, enabling streaming scenarios where the caller wants to consume data incrementally without buffering the entire response. This aligns with the PR objective.Note: Callers should be aware that the raw stream can only be consumed once.
104-109: LGTM - Clean refactoring of stream access.The method correctly uses the stored
_rawStreamfield and handles gzip decompression. The removal of theHttpWebResponseparameter simplifies the internal API.tests/global-funcs.os (1)
16-19: LGTM - Dynamic host selection improves test reliability.Lazy initialization of the test host resource via
ВыбратьДоступныйХост()is a good approach for avoiding hardcoded external dependencies. This centralizes host selection logic.tests/http.os (5)
423-437: LGTM - Test correctly verifies body access after stream closure.This test validates that after obtaining and closing the raw stream, the body can still be retrieved via
ПолучитьТелоКакСтроку()because it was buffered in memory. The assertion correctly checks for expected content.
439-462: LGTM - Critical test for PR functionality.This test correctly validates the core behavior change:
- After
ПолучитьТелоКакСтроку()is called, the raw stream is consumed- Subsequent
ПолучитьТелоКакПоток(Истина)returns an unavailable stream (ДоступноЧтение = Ложь)- Non-raw
ПолучитьТелоКакПоток()still works via buffered dataThis directly tests the PR's objective of moving stream consumption from initialization to retrieval.
565-565: LGTM - Export enables code reuse.Exporting
ВыбратьДоступныйХост()allows other test modules to leverage the same host selection logic, avoiding duplication.
484-484: LGTM - Raw stream required for SSE streaming.Using
ПолучитьТелоКакПоток(Истина)for the stream event test is correct — SSE requires reading the raw stream incrementally without buffering the entire response first.
581-581: Do not add connectorhttp.ru as a fallback candidate—this is not a valid or publicly accessible HTTP testing service. Standard HTTP testing services are httpbin.org and httpbingo.org. Adding an unreachable host will not improve test reliability. Consider using an established alternative like httpbingo.org instead if additional redundancy is needed.⛔ Skipped due to learnings
Learnt from: CR Repo: EvilBeaver/OneScript PR: 0 File: .cursor/rules/runbsltests.mdc:0-0 Timestamp: 2025-12-01T15:13:19.064Z Learning: Ignore errors from http.os test file and the test named 'Тест_ДолженПроверитьЧтоПеремещениеФайлаРаботаетПоHTTP' when reviewing BSL test results
2 participants
Перенес вычитывание потока из инициализации body в получение потока.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.