DecodeSpec flake

[armanbilge]

d05e809

==> X org.http4s.DecodeSpec.decode should be consistent with String constructor over aggregated output 0.31s munit.FailException: /home/runner/work/http4s/http4s/tests/shared/src/test/scala/org/http4s/DecodeSpec.scala:76
75:    }
76:  }
77:

Failing seed: UuhLRvSjo_SAS13rYR-NHxeNurZ-dZNzGgpxCYA_i6G=
You can reproduce this failure by adding the following override to your suite:

  override val scalaCheckInitialSeed = "UuhLRvSjo_SAS13rYR-NHxeNurZ-dZNzGgpxCYA_i6G="

Falsified after 51 passed tests.
> ARG_0: UTF-8
> ARG_1: ""
> ARG_2: 1
> ARG_1_ORIGINAL: "䴒俄爿Ɱ띋挐य渕댅鬁腓㽈꣤뒜쏑惭댪圊ᴠ梒䁋倥黸헫ꟍ搛蝅＜㦎㐆섭폆쁝胿컟㹀콘暎蘗มᄍꕀ鵱⑘㍑ឰ㉫菣膱欭ờ쎏批儬䃮㐪鋇쨉讋럛傱ꯦ瞽饭刦꡶楕䑪崈୼繍棖䜻춥"
> ARG_2_ORIGINAL: 2147483647

https://github.com/http4s/http4s/actions/runs/6009428051/job/16298838183?pr=7255

[eshu]

@armanbilge There should be def, otherwise the initialization error happens:

override def scalaCheckInitialSeed = "UuhLRvSjo_SAS13rYR-NHxeNurZ-dZNzGgpxCYA_i6G="

And after that bug is not reproducible (had been fixed?)...

[armanbilge]

@eshu Thanks for looking! Can you reproduce the bug with the exact commit I mentioned?

[eshu]

@armanbilge I replaced content of flake.lock with this one: https://github.com/http4s/http4s/blob/d05e8097facfa3f25fcd527d9032ee4c768260ad/flake.lock
and the build was succeed with the line

  override def scalaCheckInitialSeed = "UuhLRvSjo_SAS13rYR-NHxeNurZ-dZNzGgpxCYA_i6G="

in DecodeSpec.

[armanbilge]

@eshu thanks for clarifying :) what happens if you check out the entire repository at d05e809 instead of just the flake.lock, can you reproduce it?

[eshu]

@armanbilge
It looks like there are many differences in dependencies. But I see

org.http4s.DecodeSpec:
  + decode should be consistent with utf8.decode 2.656s
  + decode should be consistent with String constructor over aggregated output 0.905s
  + decode should decode an empty chunk 0.051s
  + decode should drop Byte Order Mark 0.003s
  + decode should handle unmappable character 0.005s
  + decode should handle overflows 0.004s
  + decode should not crash in IllegalStateException 0.003s
  + decode stream result should be consistent with nio's decode on full stream 0.087s

$ git show HEAD
commit c2e412345fd9eb4c67355f7ab6c0f3ff33312d52 (HEAD)
Author: http4s-steward[bot] <106843772+http4s-steward[bot]@users.noreply.github.com>
Date:   Tue Aug 29 07:50:19 2023 +0000

    flake.lock: Update
    
    Flake lock file updates:
    
    • Updated input 'typelevel-nix':
        'github:typelevel/typelevel-nix/ed9f189d304aa85cb74c052d063dfc11023b4f33' (2023-08-21)
      → 'github:typelevel/typelevel-nix/b1b16aaf47198209bf91d41a64007c5a39c02a13' (2023-08-28)
    • Updated input 'typelevel-nix/devshell':
        'github:numtide/devshell/d208c58e2f7afef838add5f18a9936b12a71d695' (2023-08-20)
      → 'github:numtide/devshell/2aa26972b951bc05c3632d4e5ae683cb6771a7c6' (2023-08-23)
    • Updated input 'typelevel-nix/flake-utils':
        'github:numtide/flake-utils/919d646de7be200f3bf08cb76ae1f09402b6f9b4' (2023-07-11)
      → 'github:numtide/flake-utils/f9e7cf818399d17d347f847525c5a5a8032e4e44' (2023-08-23)
    • Updated input 'typelevel-nix/nixpkgs':
        'github:nixos/nixpkgs/3476a10478587dec90acb14ec6bde0966c545cc0' (2023-08-20)
      → 'github:nixos/nixpkgs/cddebdb60de376c1bdb7a4e6ee3d98355453fe56' (2023-08-27)

$ git diff
diff --git a/tests/shared/src/test/scala/org/http4s/DecodeSpec.scala b/tests/shared/src/test/scala/org/http4s/DecodeSpec.scala
index 16f044b0c9..ad0eec0b36 100644
--- a/tests/shared/src/test/scala/org/http4s/DecodeSpec.scala
+++ b/tests/shared/src/test/scala/org/http4s/DecodeSpec.scala
@@ -33,6 +33,8 @@ import java.nio.charset.{Charset => JCharset}
 import scala.util.Try
 
 class DecodeSpec extends Http4sSuite {
+  override def scalaCheckInitialSeed = "UuhLRvSjo_SAS13rYR-NHxeNurZ-dZNzGgpxCYA_i6G="
+
   test("decode should be consistent with utf8.decode") {
     forAll { (s: String, chunkSize: Int) =>
       (chunkSize > 0) ==> {

Is it correct or I did something wrong?

[armanbilge]

what happens if you check out the entire repository at d05e809

@eshu, sorry I should have clarified: what I mean by "checkout" is if you do

git checkout d05e8097facfa3f25fcd527d9032ee4c768260ad

And then fix the seed, like you did above.

d05e809 was the commit that originally had the flake, so we should make sure that we can still reproduce that.

Then, if d05e809 is broken, but as you say the current code is working, we may want to do a git bisect to try and find out how or why it is fixed.

Let me know if that makes sense! Thanks for all your work.

[eshu]

@armanbilge Sorry, I am not very good with git, and I see the error I've never seen before

$ git checkout d05e8097facfa3f25fcd527d9032ee4c768260ad
fatal: reference is not a tree: d05e8097facfa3f25fcd527d9032ee4c768260ad

Also there is the message on the page d05e809:
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
I don't know what I can do with it, where I can find this fork...

[armanbilge]

@eshu here, I created a branch with that commit, that should make it easier :)

https://github.com/http4s/http4s/tree/issue/7256

[armanbilge]

Oh, I just realized maybe there is some confusion ... I'm so sorry!

The title of this issue is "DecodeSpec flake". In this context a "flake" refers to a test that is "flaky", because it is failing sometimes.

(of a device or software) prone to break down; unreliable.

---

In the repository, we also have a flake.nix and flake.lock. This is a Nix flake for the development environment and is almost definitely not related to this issue at all.

[eshu]

So DecodeSpec could be passed successfully even with the initial seed your mentioned before?

The problem was not reproduced even with branch you kindly made for me...

Sorry, I am completely new in this community, just want to learn more, this is why so many questions and misunderstandings :(

[armanbilge]

Sorry, I am completely new in this community, just want to learn more, this is why so many questions and misunderstandings :(

Please don't apologize, and ask as many questions as you need to! We are happy to have you here :)

Sometimes things are confusing and I don't realize right away, like about "flake".

---

The problem was not reproduced even with branch you kindly made for me...

Hmm, thanks for trying that. Until we can reproduce the bug successfully, it's not safe to make any assumption whether it is fixed or not.

Instead of using the seed, let's try this. I see that the test output reported the inputs that caused the failure. So maybe we can manually create a test for those arguments and see what happens. Do you think you can give that a try?

Falsified after 51 passed tests.
> ARG_0: UTF-8
> ARG_1: ""
> ARG_2: 1

Btw, feel free to open a draft PR with the test you add. This will make it easier for us to follow your progress and give help.

[eshu]

@armanbilge If you do not mind I will push changes to your branch. I applied exact parameters as you told me, but...

[armanbilge]

@eshu sure, go ahead! I don't think you can push directly to that branch, but you can base off of it and push to your fork.

[eshu]

The PR is here: #7301
It looks like it passed JVM tests, but failed for JS and native builds. It is interesting because the test with the same fixed arguments was not failed...

The problem is that I can't run native tests on my PC, they are failed with OOM.

JS tests does not work probably because I did not install node.

[armanbilge]

@eshu nice work! It looks like this bug is specific to JS and Native. Because the behavior is not matching the JVM, it's possible that this is actually not an http4s bug, but a bug in Scala.js and Scala Native. The next step would be to try and make a minimal example that shows how the behavior differs on JVM vs JS/Native.

[eshu]

@armanbilge
JS failure is:

[warn] In the last 10 seconds, 5.657 (56.6%) were spent in GC. [Heap: 0.12GB free of 6.00GB, max 6.00GB] Consider increasing the JVM heap using `-Xmx` or try a different collector, e.g. `-XX:+UseG1GC`, for better performance.
java.lang.OutOfMemoryError: Java heap space
  | => eat java.base/java.util.Arrays.copyOf(Arrays.java:3537)
  | => tat org.scalajs.linker.backend.javascript.ByteArrayWriter.ensureCapacity(ByteArrayWriter.scala:31)
        at org.scalajs.linker.backend.javascript.ByteArrayWriter.write(ByteArrayWriter.scala:52)
	at org.scalajs.linker.backend.javascript.ByteArrayWriter.write(ByteArrayWriter.scala:48)

Native failure is:

16.398s][warning][gc,alloc] pool-8-thread-25: Retried waiting for GCLocker too often allocating 256 words
[warn] In the last 10 seconds, 5.406 (55.1%) were spent in GC. [Heap: 0.06GB free of 6.00GB, max 6.00GB] Consider increasing the JVM heap using `-Xmx` or try a different collector, e.g. `-XX:+UseG1GC`, for better performance.
java.lang.OutOfMemoryError: Java heap space
	at scala.scalanative.nir.Mangle$.apply(Mangle.scala:20)
	at scala.scalanative.nir.Sig$Unmangled.mangled(Sig.scala:60)
	at scala.scalanative.nir.Sig.toProxy(Sig.scala:11)
	at scala.scalanative.linker.Reach.$anonfun$reachAllocation$9(Reach.scala:444)
	at scala.scalanative.linker.Reach$$Lambda$12665/0x00007fc2ed628000.apply(Unknown Source)

Also native throws a lot of warnings like

[warn] Cannot find brew-installed .
[warn] Cannot find brew-installed .
[warn] Cannot find brew-installed .
[warn] LD_LIBRARY_PATH will not be auto-configured.
[warn] LD_LIBRARY_PATH will not be auto-configured.
[warn] LD_LIBRARY_PATH will not be auto-configured.

I will try to figure out what is going on, but id you have any hints please tell.

[armanbilge]

@eshu it looks like you are running out of memory. Try running sbt with more memory e.g. sbt -J-Xmx12G.

[eshu]

Yes, just figured it out. Looks like "12Gb ought to be enough for anybody" 🤣
And it was failed on rootJS, I will continue the investigation

[eshu]

@armanbilge

1. String generator generates completely different sets of strings for JVM and JS for the same seed.
2. In JS some strings (I think about half) decoded to bytes contains BOM (EF BB BF) sequence ("\uFEFF"), and the decoded string does not contain it. JVM strings do not contain BOM.
3. Native generates exactly the same string like JS and fails for the same reason.

It looks like decoding happens in fs2 (at least the whole line

val decoded = source.through(decodeWithCharset[Fallible](cs.nioCharset)).compile.string

contains only fs2 calls).

I added the case to the PR: https://github.com/http4s/http4s/pull/7301/files#diff-2f31accbcdaf36e1daabdefba980864af96c94b0cd2efc1f7cfeacbd6a8ea70fR81

[armanbilge]

1. String generator generates completely different sets of strings for JVM and JS for the same seed.

Huh, that's strange. That seems like a bug of its own ...

---

In any case, nice job narrowing that down! If I understand correctly, FS2 is stripping the BOM, but the JDK is not?

I think FS2 is doing the right thing here by stripping the BOM. So maybe we need to adjust the assertion in our test to account for that as well. I wonder why the JDK doesn't strip the BOM 🤔

[eshu]

@armanbilge JVM tests does not receive BOM at all. JVM and JS generates strings for tests differently. As I show in the additional test, if BOM string is passed forcible in JVM, it is striped as well.

I can stripe BOM bytes from the test expected string if you approve it.

[armanbilge]

I can stripe BOM bytes from the test expected string if you approve it.

Yes, if you don't mind, you can propose a fix in your PR exactly like that. I think it makes sense, but I need to take a closer look, and I'd like a second opinion from another maintainer.

[eshu]

@armanbilge

I fixed the old test and wrote the new one, and I found the simple test for BOM there: https://github.com/http4s/http4s/blob/series/0.23/tests/shared/src/test/scala/org/http4s/DecodeSpec.scala#L90
Looks like the behavior of stream is expected, but Java string does not stripe BOM (maybe to make constructor and getBytes method reversible).

I just think: what if somebody creates .sbtopts file and put there memory settings? It is confusing for the first run to get OOM on the just cloned repository.

[armanbilge]

I just think: what if somebody creates .sbtopts file and put there memory settings?

We already have a .jvmopts file:

http4s/.jvmopts

Lines 1 to 6 in 778f3d0

	-Dfile.encoding=UTF8
	-Xms1G
	-Xmx6G
	-XX:ReservedCodeCacheSize=250M
	-XX:+TieredCompilation
	-XX:+UseG1GC

That configures the maximum memory that we can use in the CI runners. I'm not sure why your system needed more memory than that ...

[eshu]

@armanbilge It looks like a compilation issue for native, it happens before tests run. Maybe because of Java version?

$ java -version
openjdk version "17.0.9-ea" 2023-10-17
OpenJDK Runtime Environment (build 17.0.9-ea+6-Ubuntu-1)
OpenJDK 64-Bit Server VM (build 17.0.9-ea+6-Ubuntu-1, mixed mode, sharing)

Or maybe I need to install something else...

For JS it happens after tests

java.lang.OutOfMemoryError: Java heap space
  | => oat java.base/java.util.Arrays.copyOf(Arrays.java:3537)
  | => eat org.scalajs.linker.backend.javascript.ByteArrayWriter.ensureCapacity(ByteArrayWriter.scala:31)
        at org.scalajs.linker.backend.javascript.ByteArrayWriter.write(ByteArrayWriter.scala:52)
	at org.scalajs.linker.backend.javascript.ByteArrayWriter.write(ByteArrayWriter.scala:48)
	at org.scalajs.linker.backend.BasicLinkerBackend$$anon$1.$anonfun$writeModuleWithSourceMap$2(BasicLinkerBackend.scala:124)
	at org.scalajs.linker.backend.BasicLinkerBackend$$anon$1.$anonfun$writeModuleWithSourceMap$2$adapted(BasicLinkerBackend.scala:123)
	at org.scalajs.linker.backend.BasicLinkerBackend$$anon$1$$Lambda$22723/0x00007f431eb0df00.apply(Unknown Source)
	at scala.collection.immutable.List.foreach(List.scala:431)
	at org.scalajs.linker.backend.BasicLinkerBackend$$anon$1.writeModuleWithSourceMap(BasicLinkerBackend.scala:123)
	at org.scalajs.linker.backend.OutputWriter.writeModule(OutputWriter.scala:73)
	at org.scalajs.linker.backend.OutputWriter.$anonfun$write$5(OutputWriter.scala:47)
	at org.scalajs.linker.backend.OutputWriter$$Lambda$22066/0x00007f431eaf5500.apply(Unknown Source)
	at org.scalajs.linker.standard.IOThrottler.throttle(IOThrottler.scala:28)
	at org.scalajs.linker.backend.OutputWriter.$anonfun$write$4(OutputWriter.scala:47)
	at org.scalajs.linker.backend.OutputWriter$$Lambda$22063/0x00007f431eaf5120.apply(Unknown Source)
	at scala.concurrent.Future$.$anonfun$traverse$1(Future.scala:850)
	at scala.concurrent.Future$$$Lambda$16895/0x00007f431df4aaa0.apply(Unknown Source)
	at scala.collection.LinearSeqOptimized.foldLeft(LinearSeqOptimized.scala:126)
	at scala.collection.LinearSeqOptimized.foldLeft$(LinearSeqOptimized.scala:122)
	at scala.collection.immutable.List.foldLeft(List.scala:91)
	at scala.concurrent.Future$.traverse(Future.scala:850)
	at org.scalajs.linker.backend.OutputWriter.$anonfun$write$3(OutputWriter.scala:46)
	at org.scalajs.linker.backend.OutputWriter$$Lambda$22058/0x00007f431eaf46a8.apply(Unknown Source)
	at scala.concurrent.Future.$anonfun$flatMap$1(Future.scala:307)
	at scala.concurrent.Future$$Lambda$12930/0x00007f431d639010.apply(Unknown Source)
	at scala.concurrent.impl.Promise.$anonfun$transformWith$1(Promise.scala:41)
	at scala.concurrent.impl.Promise$$Lambda$12931/0x00007f431d6393f0.apply(Unknown Source)
	at scala.concurrent.impl.CallbackRunnable.run(Promise.scala:64)
	at java.base/java.util.concurrent.ForkJoinTask$RunnableExecuteAction.exec(ForkJoinTask.java:1395)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:373)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1182)
	at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1655)
[info] Passed: Total 9, Failed 0, Errors 0, Passed 9